596822 - Web Console menu item check mark is inconsistent among platforms and broken on Windows

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[Patrick Walton (:pcwalton)]

Currently, the Web Console menu item has a check mark next to it on Windows indicating whether it's open or closed (although it's currently broken and it won't update when you close it via the X button). Other platforms, as well as the alt menu, don't have the check mark.

I'm slightly in favor of eliminating the check mark entirely, since as far as I can see the check mark is only used for items under the "View" and "Window" menu, not for "Tools". But I'd be glad to get UI feedback on this. The options are:

(1) Check mark
(2) "Show Web Console" which changes to "Hide Web Console"
(3) No check mark

In any case, the behavior should be fixed on Windows at least. Because this is a glaringly obvious UI issue affecting the main menu on Windows, I'm nominating this as a Firefox 4 blocker.

Comment 1

[Rob Campbell [:rc] (:robcee)]

if this is just the App Menu's Web Console menu-item, it's probably a simple fix to update it properly on show/hide.

Comment 2

[Mihai Sucan [:msucan]]

Attachment: proposed fix

This is the proposed fix. I suggest we remove the checkbox for the same reasons as you do, but also because the rest of the tools, like Downloads, Error console, or even Preferences ... do not have a checkbox, nor do they have menuitems that change their labels based on state (eg. "Show downloads" and "Hide downloads").

This patch only seems to need to fix the new app menuitem for the Web Console. I couldn't test this patch on my Windows 7 virtual machine, make fails.

Please let me know if other changes are needed. Thanks!

Comment 3

[Patrick Walton (:pcwalton)]

Comment on attachment 478534 [details] [diff] [review]
proposed fix

One line patches are delicious.

Comment 4

[Mihai Sucan [:msucan]]

Comment on attachment 478534 [details] [diff] [review]
proposed fix

Thanks Patrick for the feedback+! Asking for review from Gavin.

Comment 5

[Rob Campbell [:rc] (:robcee)]

pushed to devtools,
changeset 7f1425dc04f6.

Comment 7

[Nochum Sossonko [:Natch]]

How is this "chekin-needed" and "uiwanted"?

Comment 8

[Rob Campbell [:rc] (:robcee)]

good question. removed ui-wanted as I don't think we need that anymore.

Comment 9

[David Dahl :ddahl]

http://hg.mozilla.org/mozilla-central/rev/e1f7d24bbb1c