Closed
Bug 571567
Opened 15 years ago
Closed 14 years ago
Toolbar menu missing checkmarks for enabled items
Categories
(Toolkit :: Themes, defect)
Tracking
()
RESOLVED
FIXED
mozilla2.0b1
Tracking | Status | |
---|---|---|
blocking2.0 | --- | final+ |
People
(Reporter: aaronmt, Assigned: dao)
References
Details
(Keywords: regression)
Attachments
(2 files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
enndeakin
:
review+
|
Details | Diff | Splinter Review |
Currently on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.3a5pre) Gecko/20100610 Minefield/3.7a5pre all enabled items are missing their associative checkmark to indicate that they are enabled. See attached screenshot where all my items are enabled.
Reporter | ||
Updated•15 years ago
|
Summary: Toolbar missing checkbox for enabled items → Toolbar menu missing checkmarks for enabled items
Assignee | ||
Comment 1•15 years ago
|
||
regression from bug 567782
Blocks: 567782
Component: Toolbars → Themes
Keywords: regression
Product: Firefox → Toolkit
QA Contact: toolbars → themes
Assignee | ||
Updated•15 years ago
|
Severity: normal → major
Assignee | ||
Comment 2•15 years ago
|
||
The toolbar context menu is using a <popup> element, which is obsolete. All these should be <menupopup>s:
http://mxr.mozilla.org/mozilla-central/search?string=%3Cpopup+&find=%2Fbrowser%2F&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
I'm guessing some/all of the ones under toolkit should be too?
There's also DOM Inspector and various tests under layout/
Assignee | ||
Comment 4•15 years ago
|
||
Yeah, I don't think there's a reason to use <popup> anywhere.
Updated•15 years ago
|
blocking2.0: --- → ?
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → dao
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•14 years ago
|
||
Attachment #452670 -
Flags: review?(gavin.sharp)
Comment 7•14 years ago
|
||
Comment on attachment 452670 [details] [diff] [review]
patch
>diff --git a/toolkit/obsolete/content/globalOverlay.xul b/toolkit/obsolete/content/globalOverlay.xul
>- <popup id="aTooltip" class="tooltip" onpopupshowing="return FillInTooltip(document.tooltipNode);" >
>+ <menupopup id="aTooltip" class="tooltip" onpopupshowing="return FillInTooltip(document.tooltipNode);">
> <label id="TOOLTIP-tooltipText" class="tooltip-label" flex="1"/>
>- </popup>
>+ </menupopup>
This isn't a menu - is menupopup really appropriate here?
I don't really know offhand whether there are any other differences between popups/menupopups that we'd need to be concerned about in the general case, but I bet Neil would, so running this by him is probably a good idea...
Attachment #452670 -
Flags: review?(gavin.sharp) → review?(enndeakin)
Comment 8•14 years ago
|
||
Comment on attachment 452670 [details] [diff] [review]
patch
> I don't really know offhand whether there are any other differences between
popups/menupopups
Only in that features added in the last few years would have only handled menupopups if they were comparing tag names.
The code here should be using <tooltip> of course. Or, is this not being used?
Attachment #452670 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 9•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/1be071f3b9cb
I made toolkit/obsolete/content/globalOverlay.xul use <tooltip>, but I don't think that code is being used.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a6
Comment 10•14 years ago
|
||
> I made toolkit/obsolete/content/globalOverlay.xul use <tooltip>, but I don't
> think that code is being used.
SeaMonkey uses globalOverlay.xul. But as far as I can tell only so that we pull in chrome://global/content/globalOverlay.js. We don't use any of the XUL markup in SeaMonkey.
Comment 11•14 years ago
|
||
(In reply to comment #3)
> I'm guessing some/all of the ones under toolkit should be too?
> There's also DOM Inspector and various tests under layout/
Ian, do you know if we have bugs around to do this change in comm-central?
Comment 12•14 years ago
|
||
Well Bug 572682 <popupset> is for suite/. This should cover the majority of cases there.
Updated•14 years ago
|
blocking2.0: ? → final+
You need to log in
before you can comment on or make changes to this bug.
Description
•