Closed Bug 583218 Opened 11 years ago Closed 10 years ago

"Web Console" menu item should be checked when the Web Console is open

Categories

(Firefox :: Menus, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 12
Tracking Status
blocking2.0 --- -

People

(Reporter: scoobidiver, Assigned: lgvalent)

References

Details

(Keywords: ux-consistency)

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre
Build Identifier: Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; rv:2.0b3pre) Gecko/20100729 Minefield/4.0b3pre

As the Web console is not a stand alone window that can be closed by a x button, there must be a V on the left of the "Web console" menu when it is opened (idem "Inspect").

Reproducible: Always
Status: UNCONFIRMED → NEW
Ever confirmed: true
Version: unspecified → Trunk
This should be a trivial thing to fix. See the solution on bug 584033. Vlado, are you interested?
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: No V on the left of "Web console" menu when it is opened → "Web console" in menu isn't checked when it is open
As of Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100824 Minefield/4.0b5pre This issue appear to be fixed.
It is not fixed.

With the classic menu, the Web console menu isn't checked at all.
With the button menu, the Web console menu is not checked according to the selected tab.
(In reply to comment #1)
> This should be a trivial thing to fix. See the solution on bug 584033. Vlado,
> are you interested?

It isn't that trivial, but still should be very easy to fix. I can take a look at this, but it won't be sooner than late this week or early next week. If someone wants to work on this, feel free to do so.
See also bug 588946.
blocking2.0: --- → ?
Keywords: ux-consistency
Duplicate of this bug: 588946
Not going to hold the release for this, blocking-. Would love to get a fix though, if you have the time now Vlado.
blocking2.0: ? → -
Also please ensure we dont have issue 1 in bug 595106
Blocks: 595106
Blocks: FirefoxButton
No longer blocks: 595106
Summary: "Web console" in menu isn't checked when it is open → "Web console" in menu isn't checked (classic) or is wrongly checked (button)
Duplicate of this bug: 595106
Depends on: 595106
Add to comment 3 :
Issue 1 of bug 595106 comment 0 : at Firefox menu it don't automatically uncheck when user close Web Console using the new [X] button.
No longer depends on: 595106
Duplicate of this bug: 595106
Duplicate of this bug: 597347
Assignee: nobody → lgvalent
Status: NEW → ASSIGNED
Comment on attachment 587429 [details] [diff] [review]
A patch with the implementation for the feature requested.

>                   <menuitem id="webConsole"
>+							type="checkbox"

nit: indentation is off (you're using tabs instead of spaces)

>+    // Fix Bug 583218 - "Web console" in menu isn't checked (classic) or is wrongly checked (button)

This comment can be omitted.

>+    let webConsoleMenuitem = chromeDocument.getElementById("Tools:WebConsole");
>+    webConsoleMenuitem.setAttribute("checked", true);

chromeDocument.getElementById("Tools:WebConsole").setAttribute("checked", "true");

>+    let webConsoleMenuitem = chromeDocument.getElementById("Tools:WebConsole");
>+    webConsoleMenuitem.setAttribute("checked", false);

chromeDocument.getElementById("Tools:WebConsole").removeAttribute("checked");
Looks good otherwise! Can you attach a new patch with these changes made? I'll review it.
Dao,
These lines:

>+    let webConsoleMenuitem = chromeDocument.getElementById("Tools:WebConsole");
>+    webConsoleMenuitem.setAttribute("checked", false);

Was changed to:
chromeDocument.getElementById("Tools:WebConsole").setAttribute("checked", false);

This suggestion doesn't work:
chromeDocument.getElementById("Tools:WebConsole").removeAttribute("checked");
Comment on attachment 587660 [details] [diff] [review]
A correction of previous patch reviewed by dao.

>--- a/browser/base/content/browser-appmenu.inc	Wed Jan 11 10:41:46 2012 -0200
>+++ b/browser/base/content/browser-appmenu.inc	Wed Jan 11 10:49:21 2012 -0200
>@@ -179,7 +179,7 @@
>         <menupopup id="appmenu_webDeveloper_popup">
>           <menuitem id="appmenu_webConsole"
>                     label="&webConsoleCmd.label;"
>-                    oncommand="HUDConsoleUI.toggleHUD();"
>+                    command="Tools:WebConsole"
>                     key="key_webConsole"/>

I missed this on the first patch. You should add type="checkbox" here, even though it seems to work without it...
Attachment #587660 - Flags: review?(dao) → review+
Comment on attachment 587677 [details] [diff] [review]
Second correction of previous patch reviewed by dao.

thanks!
Attachment #587677 - Flags: review?(dao) → review+
Attachment #587429 - Attachment is obsolete: true
Attachment #587429 - Flags: review?
Attachment #587660 - Attachment is obsolete: true
http://hg.mozilla.org/integration/mozilla-inbound/rev/a157f6aab51b
Summary: "Web console" in menu isn't checked (classic) or is wrongly checked (button) → "Web Console" menu item should be checked when the Web Console is open
Target Milestone: --- → Firefox 12
https://hg.mozilla.org/mozilla-central/rev/a157f6aab51b
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 719850
Scoobidiver, We are working to fix the bug 719850, I've submitted a patch for review.
You need to log in before you can comment on or make changes to this bug.