Shoutbox

Mattike, I need you to translate "Check now" in Ducth for the next version.

Isn't it something like "Controle nu"?

In English, things like that (command verbs) are always in the infinitive form. Many people forget this and interpret the English captions wrong when they translate...

"Check Now" is not "controleer nu" (first person), but "nu controleren" (infinitive)....

Though it depends on what "Check Now" stands for. Checking for updates ("controleren op updates")? Checking something cool out ("zie dit" or something like that... sounds dodgy though)? Checking a setting ("nu selecteren")? etc...

When you need something translated, _always_ place it in the correct context since stuff can rarely be translated literally and still be correct in the context. Also the size available is of importance. Aka: requesting something to be translated without these two important things is useless.

quote:

---

Though it depends on what "Check Now" stands for. Checking for updates ("controleren op updates")? Checking something cool out ("zie dit" or something like that... sounds dodgy though)? Checking a setting ("nu selecteren")? etc...

---

"Check for new private messages."

quote:

---

Originally posted by mickael9

quote:

---

Though it depends on what "Check Now" stands for. Checking for updates ("controleren op updates")? Checking something cool out ("zie dit" or something like that... sounds dodgy though)? Checking a setting ("nu selecteren")? etc...

---

"Check for new private messages."

---

Then I would say "Nu controleren".

-------------

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.

-------------

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.

-------------

Other stuff to optimize:

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

-------------

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.

-------------

Instead of using == and != here and there, use === and !==.
See JScript 5.6 documentation or one of my forum posts why.

-------------

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

-------------

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.

-------------

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.

-------------

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).

-------------

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();
}

---

Same goes for the event OnEvent_MenuClicked.

-------------

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>';
}

---

-------------

if (objFileName.match(/^(.*)\.xml$/i))
=>
if (objFileName.match(/^(.+)\.xml$/i))

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

0.5 is out

quote:

---

Originally posted by mickael9

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 ^^

---

Be very carefull in that. Before you know it you write a string instead of a dword or vice versa, etc because you made an error somewhere in your code or whatever. And before you know it you end up with settings which can't be read properly anymore by users who don't have the latest version or by users which has bad settings already in the registry. Then you need to compensate for that in another version, etc... big mess...

The reason to add a third parameter is so you can explicitly set a type, even if you screwed up somewhere in the script or Jscript's registry read function read the wrong type of variable (upon its return you expect it to be a certain type, but you can't be sure. And if you don't check upon it it is very well possible people will encounter errors in the script, etc...)

Types of variables in JScript can switch and change as you go, the benefit of adding a third parameter to this function is exactly to overcome this and write the boolean true or the string "true" as a DWORD... 0x1 for example.

If you're going to auto-detect it, you're better of not changing anything to what it was as auto-detecting will cause more potential errors to happen.

All this is also related to the use of operators like === instead of ==, etc...

quote:

---

Originally posted by mickael9

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 ?

---

You miss the point here. See JScript 5.6 documentation or one of my forum posts why.

quote:

---

Originally posted by mickael9

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".

---

All that has got nothing to do with the fact that you don't store, read and handle settings user-specific.

quote:

---

Originally posted by mickael9

quote:

---

Same goes for the event OnEvent_MenuClicked.

---

In the next version, there is a new menu : "Check now"

---

In that case I'll sush

PS: but if that was why you needed the translation, I would translate it like "Controleren op nieuwe prive berichten" (but this is of course very long).

Otherwise the user might mistaken it for checking on an update of the script.
The same goes for the English version, Don't just say "Check Now", but "Check on new private messages"

So you see, placing things in exact and detailed context and where it is going to be used exactly is extremely important in translating, just as what the translation means (eg: "check on new pms") and what is shown (eg: "Check now") can be/mean a huge difference.

Hello!

I have a question:

Can I make this script for another forum and can you tell me how?
Do I change the url of all things, beeing in your script and change it with my forum-url?!