Closed
Bug 643184
Opened 14 years ago
Closed 10 years ago
Checkbox menu items in a toolbar do not display the check mark when checked in Firefox 4 on Mac OS X
Categories
(Toolkit :: Themes, defect)
Tracking
()
RESOLVED
FIXED
mozilla33
Tracking | Status | |
---|---|---|
firefox5 | - | --- |
People
(Reporter: bugzilla, Assigned: stefanh)
References
Details
(Keywords: regression)
Attachments
(3 files, 2 obsolete files)
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0) Gecko/20100101 Firefox/4.0
Checkbox menu items in a toolbar do not display the check mark when checked in Firefox 4 on Mac OS X. They do seem to work in a menu rather than a toolbar - using the DOM Inspector it appears to be because the iconic part of the menu has a width of 0, but I'm not sure where this is coming from.
Note that the exact same code works in Firefox 3 and earlier so this appears to be a regression (or change) in Firefox 4.
Reproducible: Always
Steps to Reproduce:
1. Install the Web Developer extension from https://addons.mozilla.org/en-US/firefox/addon/web-developer/ and restart
2. Go to Google.com and click 'Outline' -> 'Outline Block Level Elements' in the Web Developer toolbar
3. Re-open the 'Outline' menu in the Web Developer toolbar
Actual Results:
The 'Outline Block Level Elements' menu is not checked
Expected Results:
The 'Outline Block Level Elements' menu should be checked
The 'Outline Block Level Elements' menu under the main browser 'Tools' menu then 'Web Developer' -> 'Outline' is checked after the steps above, but I'm not sure why that is behaving differently. Again, note that this all works in Firefox 3.
Reporter | ||
Comment 1•14 years ago
|
||
Sorry, forgot to also mention that these same steps work just fine on Windows so this appears to be specific to Mac OS X.
Comment 2•14 years ago
|
||
Regression between 10090103 and 10090203.
PASS: http://hg.mozilla.org/mozilla-central/rev/f47972d05473
FAIL: http://hg.mozilla.org/mozilla-central/rev/dc2939f2640d
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f47972d05473&tochange=dc2939f2640d
Not sure which of those changesets caused this regression.
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Almost all my stuff was backed out in that interval. I don't know which of the rest could have caused this.
Comment 4•14 years ago
|
||
Sadly I cannot build any version of Minefield from this given range due to a bustage in Spidermonkey.
Alice, are you able to do builds to get a precise changeset for a regression?
Comment 6•14 years ago
|
||
(In reply to comment #5)
> Alice, are you able to do builds to get a precise changeset for a regression?
I do not have Mac.
Comment 7•14 years ago
|
||
Markus, are you able to compile the builds from my given regression range in comment 2? Sadly I still have no success.
Comment 8•14 years ago
|
||
We just encountered this bug in our Page Saver add-on so it may be affecting a lot of different toolbar-based menus on Mac OS. Also, bug 642112 looks like it might be a duplicate.
Josh, this regression sounds kinda bad, maybe you can look into it?
Assignee: nobody → joshmoz
Comment 10•14 years ago
|
||
I'm not sure the regression range in comment #2 is accurate. I did a build of f47972d05473 and I'm seeing the problem there following the STR given.
Actually, it's not 100% reliable; at one point, after playing around with a number of the Web Developer toolbar menus, suddenly the checkmarks in the Outline menu began to appear as expected. After restarting the browser, they're missing again, and I have not found a way to restore them. But there's something intermittent/flaky about this....
Comment 11•14 years ago
|
||
Testing with nightlies indicates a regression in the 2010-06-23-03-mozilla-central build; 06-22 works fine.
This points to a regression range of:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ce18b4287791&tochange=8f05ab3aa198
Note that in builds after the regression, the checkmarks still _sometimes_ show up correctly, so repeated testing is needed in order to be reasonably confident whether a given build has the problem or not.
Comment 12•14 years ago
|
||
Bug 559033 jumps out at me in that regression range:
Markus Stange: [Mac] New toolbar button style
Comment 13•14 years ago
|
||
Reassigning to Markus for now since it looks like his regression. If Markus can't look at it we can find a new owner, possibly me.
Assignee: joshmoz → mstange
Comment 15•14 years ago
|
||
Dao probably knows this stuff well, too.
Comment 17•14 years ago
|
||
Looks like we should track this regression for Firefox 5 inclusion. It's a highly visible regression on OS X.
tracking-firefox5:
--- → ?
Comment 18•14 years ago
|
||
We wouldn't block going to beta or a release on this issue so we are not tracking for 5
Christian, we need a way to track Firefox 4 regressions that we should fix in Firefox 5. What do you suggest?
Comment 20•14 years ago
|
||
Here the comment from Jorge on bug 651008, which will be a dupe of this bug but explains the problem in detail:
Forgot to mention the cause: the menuitem element has a child box that has its
background-image property set to the check icon. For some reason the width of
this box is set to 0 by default, except for the main menuitems as mentioned in
comment #2. Setting the width to some other value with CSS fixes the problem.
Component: Menus → Theme
QA Contact: menus → theme
Comment 22•14 years ago
|
||
(In reply to comment #19)
> Christian, we need a way to track Firefox 4 regressions that we should fix in
> Firefox 5. What do you suggest?
Christian, any comment from you on that topic?
Beside that question we could simply come up with a fix and ask for approval-aurora once it has been landed on m-c.
Comment 23•14 years ago
|
||
I've done another regression search using the testcase in bug 651008 and have come to this regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=3e7755e00fa1&tochange=32a13ebe9ba0 which points to bug 542192
Blocks: 542192
Updated•14 years ago
|
Component: Theme → Themes
Product: Firefox → Toolkit
QA Contact: theme → themes
Comment 24•13 years ago
|
||
I tried to install a different theme (instead of the default one) ad the problem is "solved". Menus work well using "MacOSX Theme (Firefox 4+) 1.7.0".
hoping this helps...
Comment 25•13 years ago
|
||
Since this hasn't been mentioned yet: here's a testcase. This appears to only happen if the menu is initially displayed with the menuitem not having a checked= attribute. If it's checked (either originally set in the xul, or set by code) when the menu is displayed, it's fine.
Comment 26•12 years ago
|
||
I have a patch that works with testcase.
Attachment #655382 -
Flags: review?(dao)
Comment 27•12 years ago
|
||
Updated to work with testcase from bug 651008.
Attachment #655382 -
Attachment is obsolete: true
Attachment #655382 -
Flags: review?(dao)
Attachment #655397 -
Flags: review?(dao)
Updated•12 years ago
|
Attachment #655397 -
Flags: review?(enndeakin)
Comment 28•12 years ago
|
||
Comment on attachment 655397 [details] [diff] [review]
patchV2
This causes a large gap to appear on the left side of items in menulists between the checkbox and label. You likely need to have the width or something else apply only to items not in menulists.
Attachment #655397 -
Flags: review?(enndeakin) → review-
Updated•12 years ago
|
Attachment #655397 -
Flags: review?(dao)
Updated•12 years ago
|
Assignee: mstange → nobody
Whiteboard: [4rc2]
Comment 29•12 years ago
|
||
Neil, can you attach an example or screenshot of that gap?
Comment 30•12 years ago
|
||
You should be able to see it in the homepage menulist in the General tab of the Preferences window.
Comment 31•12 years ago
|
||
Attachment #655397 -
Attachment is obsolete: true
Attachment #665093 -
Flags: review?(enndeakin)
Comment 32•12 years ago
|
||
The following testcase shows various combinations of iconic and checkbox menuitems a toolbar.
I'm not sure if this patch handles the case where both a set properly. Note the margins on the left for example.
The needed patch should ideally preserve the existing behaviour. (Or it can implement the actual Mac native look which isn't exactly what we do currently)
Comment 33•12 years ago
|
||
Comment on attachment 665093 [details] [diff] [review]
patchV3
Clearing review due to issues noted above.
Attachment #665093 -
Flags: review?(enndeakin) → review-
Comment 34•12 years ago
|
||
Komodo bug http://bugs.activestate.com/show_bug.cgi?id=96556 depends on this.
Comment 35•11 years ago
|
||
The problem still exists in FF 23 on Mac and prevents the correct rendering of toolbarbutton > menupopup > menuitem[type="checkbox"]. If the checkmark was not set during the first popup event, it will never be displayed.
I investigated the problem a little and it seems to have to do with the way the outset indentation for the checkmark is defined. The space reserved by .menu-iconic-left seems to be too small (15px) for the typical icon (16px). The icon itself also misses a width declaration.
The following alternative definition fixes the problem for me:
.menu-iconic-left { margin-left: -16px; margin-right: -16px; padding-left: 16px; }
.menu-iconic-icon { width:16px; margin-left: 0px; margin-right: 0px; }
Comment 36•11 years ago
|
||
(In reply to Ralf Strobel from comment #35)
> The problem still exists in FF 23 on Mac and prevents the correct rendering
> of toolbarbutton > menupopup > menuitem[type="checkbox"]. If the checkmark
> was not set during the first popup event, it will never be displayed.
>
> I investigated the problem a little and it seems to have to do with the way
> the outset indentation for the checkmark is defined. The space reserved by
> .menu-iconic-left seems to be too small (15px) for the typical icon (16px).
> The icon itself also misses a width declaration.
>
> The following alternative definition fixes the problem for me:
> .menu-iconic-left { margin-left: -16px; margin-right: -16px; padding-left:
> 16px; }
> .menu-iconic-icon { width:16px; margin-left: 0px; margin-right: 0px; }
Thanks! I can confirm Ralf's fix works for me (FF 24 on OSX 10.8.5).
Comment 37•11 years ago
|
||
Ralf, could you provide a patch and ask for review? Thanks!
Comment 38•11 years ago
|
||
Sorry, I don't work on FF itself, just add-ons. I don't even know where the original theme is defined.
Assignee | ||
Comment 39•11 years ago
|
||
Bug 1012445 will fix this since we'll then draw a native checkmark over the menuitem instead of setting an icon with css.
Depends on: 1012445
Assignee | ||
Comment 40•10 years ago
|
||
This was fixed in bug 1012445.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Assignee: nobody → stefanh
Target Milestone: --- → mozilla33
You need to log in
before you can comment on or make changes to this bug.
Description
•