quote:
btw, can I suggest another way of logging in?
The current method logs people out, even if they don't wanna. Also, all the steps you do aren't needed.
Just go directly to "http://shoutbox.menthix.net/private.php?fid=1" and if a page appears that the user isn't logged in, only then explicitly log in (and back out later). If that page isn't shown, the user is already logged in and you will go directly to the inbox. In the latter case, you should not log out either.
OK ...
quote:
Another suggestion: The title of the script is "MsgHelp Private Message Notifier". This is too long to be shown in the script list. Also you use "MsgHelp PM Notifier" as title for the prefs window, which is inconsistant with the script's title. So, to strike two flies at the same time, I would stick to "MsgHelp PM Notifier" for the script title and message boxes, etc too.
OK ...
quote:
code:
String.prototype.isEmpty = function()
{
if (this.replace(/^ +/, '').replace(/ +$/, '') == '')
return true;
return false;
}
To trim a string from spaces, you can use this.replace(/^ +| +$/g, '')
Also, you don't need an IF THEN if you directly return true or false according to the check.
thus:code:
String.prototype.isEmpty = function()
{
return (this.replace(/^ +| +$/g, '') === '')
}
(and instead of a space, maybe better use a spacing character which is \s
OK ...
quote:
In the Settings.Set prototype I would suggest to add a third parameter which defines the type of registry key you're going to write. As it is now, you only write strings, but CheckInterval and Enable are better to be written as DWORDs.
I think I will detect the type of the variable instead ^^
quote:
Instead of using == and != here and there, use === and !==.
See JScript 5.6 documentation or one of my forum posts why.
Hum ... but String.replace returns a string, so I don't need === no ?
quote:
I see you check on no/empty input and then set the focus to that control... All I can say is 
for this. Most complete and user friendly checking I've seen.
You also don't write settings to the registry when it is not needed (eg: when starting up or when user hasn't changed anything), also (some scripts do write (default) stuff to the registry, eg: when they can't find a setting, which is 
).
Example for all 
quote:
Related to the above, check if the CheckInterval isn't greater than
86400000/60/1000 = 1440, since a timer value can't be bigger than 86400000.
Don't forget to change function SettingsOK too.
True ...
quote:
code:
function Logout_Callback()
{
if (objXmlHttp.readyState == 4)
{
Login();
}
}
Isn't that going to give an endless loop when there is some fault where the user of the script isn't the user who was logged in??? trying to log in, different user detected, log out, trying to log in, different user detected, log out, trying to log in, etc...
I can understand you first log out and then try to log in with the correct user, but the way it is implemented will also make that you constantly loop when the logging in fails and when again another user is shown.
Either do it differently, or make it only check a certain amount of times and then showing an error.
Mhhhh ...
quote:
code:
function OnEvent_Initialize(blnMessengerStart)
{
LoadStrings();
if (SettingsOK())
StartTimerAndCheck();
else
ShowConfigDialog();
}
Do not use OnEvent_Initialize to load the settings. Use OnEvent_SignIn or even better OnEvent_SigninReady which makes your script user specific (also store your settings user specific!!!), which is very very very mandatory for something like this!!! (And don't forget to cancel the timer when the user signs out of course).
I will allow the user to leave the fields blank when the "Enable" checkbox is disabled
Maybe change the label to "Enable for the current account"
quote:
code:
function OnEvent_Timer(strTimerId)
{
if (strTimerId == 'CheckTimer')
{
StartTimerAndCheck();
}
}
Since you only use one timer, you don't need to check upon the timer id.
And thus:code:
function OnEvent_Timer()
{
StartTimerAndCheck();
}
OK ...
quote:
Same goes for the event OnEvent_MenuClicked.
In the next version, there is a new menu : "Check now"
quote:
code:
function OnGetScriptMenu(intLocation)
{
var strMenu = '<ScriptMenu>';
strMenu += '<MenuEntry Id="mnuSettings">' + Lang.Settings + '</MenuEntry>';
strMenu += '</ScriptMenu>';
return strMenu;
}
No need for the variable and parameters:code:
function OnGetScriptMenu()
{
return '<ScriptMenu>'
+ '<MenuEntry Id="mnuSettings">' + Lang.Settings + '</MenuEntry>'
+ '</ScriptMenu>';
}
Right
quote:
if (objFileName.match(/^(.*)\.xml$/i))
=>
if (objFileName.match(/^(.+)\.xml$/i))
Oh yes
/ 
