Closed Bug 1497328 Opened Last year Closed Last year

MessageWindowController's isCommandEnabled() contains duplicate switch cases

Categories

(Thunderbird :: Mail Window Front End, enhancement, minor)

enhancement
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 64.0

People

(Reporter: jsbruner, Assigned: jsbruner)

Details

Attachments

(1 file)

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
Attached patch bug1497328.patch β€” β€” Splinter Review
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?
Flags: needinfo?(geoff)
Keywords: checkin-needed
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
Flags: needinfo?(geoff)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3402a188cb8e
Remove duplicate switch cases from MessageWindowController's isCommandEnabled(). r=aceman
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
You need to log in before you can comment on or make changes to this bug.