Closed
Bug 404229
Opened 17 years ago
Closed 16 years ago
remove inclusion of global/skin/toolbar.css from browser.xul
Categories
(Toolkit :: UI Widgets, defect)
Toolkit
UI Widgets
Tracking
()
RESOLVED
FIXED
mozilla1.9.2a1
People
(Reporter: rflint, Assigned: dao)
References
Details
(Keywords: perf)
Attachments
(1 file, 2 obsolete files)
5.30 KB,
patch
|
Gavin
:
review+
|
Details | Diff | Splinter Review |
Bug 250867 was on the right track by making the mode styling a feature of toolbar widgets, but a bit off in that it required all consumers to include toolbar.css to make use of these features. It would be cleaner to just make toolbarbuttons aware of what their parents are up to.
This also includes a fix to get rid of editMenuOverlay.xul's unnecessary use of the global skin.
Attachment #289209 -
Flags: review?(gavin.sharp)
Updated•17 years ago
|
Version: unspecified → Trunk
Reporter | ||
Comment 1•16 years ago
|
||
This updates the patch to trunk and since there are no problems caused by leaving the toolbar.css declarations around, I've simply removed the changes to all other products except Firefox.
Attachment #289209 -
Attachment is obsolete: true
Attachment #342012 -
Flags: review?(gavin.sharp)
Attachment #289209 -
Flags: review?(gavin.sharp)
Reporter | ||
Updated•16 years ago
|
Attachment #342012 -
Flags: review?(gavin.sharp) → review?(mano)
Comment 2•16 years ago
|
||
Comment on attachment 342012 [details] [diff] [review]
Updated patch
r=me, but you should at least file bugs on the other projects affected by bug 250867.
Attachment #342012 -
Flags: review?(mano) → review+
Reporter | ||
Comment 3•16 years ago
|
||
Comment on attachment 342012 [details] [diff] [review]
Updated patch
Requesting approval for a safe, small Ts/Txul win that should land for beta 2 as it'll require minimal action (appropriately documented on devmo, of course :) from third-party themes.
Attachment #342012 -
Flags: approval1.9.1b2?
Comment 4•16 years ago
|
||
Comment on attachment 342012 [details] [diff] [review]
Updated patch
a191b2=beltzner; yeah, good to get this for b2
Attachment #342012 -
Flags: approval1.9.1b2? → approval1.9.1b2+
Reporter | ||
Comment 5•16 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Reporter | ||
Updated•16 years ago
|
Target Milestone: --- → mozilla1.9.1b2
Updated•16 years ago
|
Keywords: dev-doc-needed,
late-compat
Reporter | ||
Updated•16 years ago
|
Keywords: dev-doc-needed → dev-doc-complete
Comment 6•16 years ago
|
||
Interesting, since dao had to put
window:not([active="true"]) menubar > menu {
color: ThreeDShadow;
}
into global.css and not menu.css so that the menu would update dynamically...
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #3)
> should land for beta 2 as it'll require minimal action
The display:none stuff looks like it belongs in xul.css anyway.
Comment 6 also concerns me.
Assignee | ||
Comment 8•16 years ago
|
||
(In reply to comment #6)
> Interesting, since dao had to put
>
> window:not([active="true"]) menubar > menu {
> color: ThreeDShadow;
> }
>
> into global.css and not menu.css so that the menu would update dynamically...
I just got to a state where I'm stuck with icons+text, i.e., neither mode=icons nor mode=text make a difference.
Assignee | ||
Comment 9•16 years ago
|
||
backed out
Comment 10•16 years ago
|
||
--> future; I'm not aware of a reason why we'd want to force this into 3.1
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Target Milestone: mozilla1.9.1b2 → Future
Updated•16 years ago
|
Attachment #342012 -
Flags: approval1.9.1b2+
Assignee | ||
Comment 11•16 years ago
|
||
Attachment #349324 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #349324 -
Flags: review? → review?(rflint)
Assignee | ||
Updated•16 years ago
|
Keywords: dev-doc-needed,
late-compat
Comment 12•16 years ago
|
||
Ugh, is there any reason why you do not remove the superfluous inclusion of global/skin in editMenuOverlay.xul in the new version? I was just about the file a bug on this when I spotted this in the logs...
Assignee | ||
Comment 13•16 years ago
|
||
(In reply to comment #12)
> Ugh, is there any reason why you do not remove the superfluous inclusion of
> global/skin in editMenuOverlay.xul in the new version?
No, I just didn't read the comments and the patch that carefully, so it got lost. I can update the patch, although filing a separate bug wouldn't be wrong anyway. (E.g. it would have stuck when the toolbar mode stuff got backed out.)
Comment 14•16 years ago
|
||
Yes, I agree, it's probably better to treat that issue independently in any case, so I filed bug 466302 for it.
Assignee | ||
Updated•16 years ago
|
Attachment #349324 -
Flags: review?(rflint) → review?(gavin.sharp)
Assignee | ||
Updated•16 years ago
|
Summary: Make toolbarbuttons aware of toolbar mode → remove inclusion of global/skin/toolbar.css from browser.xul
Comment 15•16 years ago
|
||
Comment on attachment 349324 [details] [diff] [review]
fixed patch
r=me. Looks like bug 465731 covers the thunderbird part, but it probably wouldn't hurt to file a bug on calendar to let them know that they might not need the toolbar.css reference anymore.
Attachment #349324 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 16•16 years ago
|
||
Assignee: rflint → dao
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Resolution: --- → FIXED
Target Milestone: Future → mozilla1.9.2a1
Assignee | ||
Updated•16 years ago
|
Attachment #342012 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•