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)

All
macOS
defect
Not set
normal

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.
Sorry, forgot to also mention that these same steps work just fine on Windows so this appears to be specific to Mac OS X.
Keywords: regression
Hardware: x86 → All
Whiteboard: [4rc2]
Version: unspecified → Trunk
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.
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?
(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.
Markus, are you able to compile the builds from my given regression range in comment 2? Sadly I still have no success.
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
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....
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.
Bug 559033 jumps out at me in that regression range: Markus Stange: [Mac] New toolbar button style
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
Dao probably knows this stuff well, too.
Looks like we should track this regression for Firefox 5 inclusion. It's a highly visible regression on OS X.
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?
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
(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.
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
Component: Theme → Themes
Product: Firefox → Toolkit
QA Contact: theme → themes
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...
Attached file Testcase (deleted) —
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.
Attached patch patchV1 (obsolete) (deleted) — Splinter Review
I have a patch that works with testcase.
Attachment #655382 - Flags: review?(dao)
Attached patch patchV2 (obsolete) (deleted) — Splinter Review
Updated to work with testcase from bug 651008.
Attachment #655382 - Attachment is obsolete: true
Attachment #655382 - Flags: review?(dao)
Attachment #655397 - Flags: review?(dao)
Attachment #655397 - Flags: review?(enndeakin)
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-
Assignee: mstange → nobody
Whiteboard: [4rc2]
Neil, can you attach an example or screenshot of that gap?
You should be able to see it in the homepage menulist in the General tab of the Preferences window.
Attached patch patchV3 (deleted) — Splinter Review
Attachment #655397 - Attachment is obsolete: true
Attachment #665093 - Flags: review?(enndeakin)
Attached file iconic and checkbox menu testcase (deleted) —
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 on attachment 665093 [details] [diff] [review] patchV3 Clearing review due to issues noted above.
Attachment #665093 - Flags: review?(enndeakin) → review-
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; }
(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).
Ralf, could you provide a patch and ask for review? Thanks!
Sorry, I don't work on FF itself, just add-ons. I don't even know where the original theme is defined.
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
This was fixed in bug 1012445.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee: nobody → stefanh
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: