The default bug view has changed. See this FAQ.

remove inclusion of global/skin/toolbar.css from browser.xul

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Toolkit
XUL Widgets
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: rflint, Assigned: dao)

Tracking

({perf})

Trunk
mozilla1.9.2a1
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 289209 [details] [diff] [review]
Patch

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)
Keywords: perf
Version: unspecified → Trunk
Created attachment 342012 [details] [diff] [review]
Updated patch

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)
Attachment #342012 - Flags: review?(gavin.sharp) → review?(mano)
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+
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 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+
http://hg.mozilla.org/mozilla-central/rev/2686d82a880c
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Keywords: dev-doc-needed, late-compat
Keywords: dev-doc-needed → dev-doc-complete

Comment 6

9 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

9 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

9 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

9 years ago
backed out
Status: RESOLVED → REOPENED
Keywords: dev-doc-complete → dev-doc-needed
Resolution: FIXED → ---
--> 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

9 years ago
Blocks: 465731
Attachment #342012 - Flags: approval1.9.1b2+
(Assignee)

Comment 11

9 years ago
Created attachment 349324 [details] [diff] [review]
fixed patch
Attachment #349324 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #349324 - Flags: review? → review?(rflint)
(Assignee)

Updated

9 years ago
Keywords: dev-doc-needed, late-compat

Comment 12

9 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

9 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

9 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

9 years ago
Depends on: 466088
(Assignee)

Updated

8 years ago
Attachment #349324 - Flags: review?(rflint) → review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Summary: Make toolbarbuttons aware of toolbar mode → remove inclusion of global/skin/toolbar.css from browser.xul
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

8 years ago
http://hg.mozilla.org/mozilla-central/rev/3778e80a2a4e
Assignee: rflint → dao
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago8 years ago
Flags: wanted1.9.2?
Flags: blocking1.9.2?
Resolution: --- → FIXED
Target Milestone: Future → mozilla1.9.2a1
(Assignee)

Updated

8 years ago
Blocks: 479674
(Assignee)

Updated

8 years ago
Attachment #342012 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.