Closed Bug 1497328 Opened Last year Closed Last year
Window Controller's is Command Enabled() contains duplicate switch cases
MessageWindowController (in messageWindow.js) has a isCommandEnabled(command) function which is more or less a large switch statement. The cases "cmd_goForward" and "cmd_goBack" appear to have been accidentally duplicated. See, for example: https://dxr.mozilla.org/comm-central/source/comm/mail/base/content/messageWindow.js#1061 and https://dxr.mozilla.org/comm-central/source/comm/mail/base/content/messageWindow.js#1051
This patch removes the duplicate switch cases. This patch DOES change the behavior of this function. Before, the first case would match and always return true. It now returns nsMsgNavigationType.back or nsMsgNavigationType.forward, depending on whether the forward or back button/command was passed.
Attachment #9015398 - Flags: review?(acelists)
Comment on attachment 9015398 [details] [diff] [review] bug1497328.patch Review of attachment 9015398 [details] [diff] [review]: ----------------------------------------------------------------- I tested this but I see these Back/Forward buttons/arrows working fine before the patch and in the standalone message window they are disabled permanently (if added to the toolbar). But the patch looks correct nevertheless. Thanks
Attachment #9015398 - Flags: review?(acelists) → review+
Darktrojan, does eslint catch this type of error?
Yes it does, it's even a recommended rule, but we'd need to have eslint enabled on that file for it to work. :) https://eslint.org/docs/rules/no-duplicate-case
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/comm-central/rev/3402a188cb8e Remove duplicate switch cases from MessageWindowController's isCommandEnabled(). r=aceman
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.