Status

()

task
RESOLVED FIXED
2 years ago
21 days ago

People

(Reporter: ntim, Assigned: timdream)

Tracking

(Blocks 1 bug)

unspecified
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
As of bug 1428938, the toolbox binding is only used for the `palette` field and has `-moz-appearance: toolbox;` associated to it.

The `palette` field can probably be folded with the CustomizableUI.jsm module, while the styles can be applied where applicable.
I'm assuming this also requires loading toolbars.css as a UA sheet so that we don't lose those styles from toolbar-base
Depends on: 1420229
Reporter

Comment 2

2 years ago
(In reply to Brian Grinstead [:bgrins] from comment #1)
> I'm assuming this also requires loading toolbars.css as a UA sheet so that
> we don't lose those styles from toolbar-base

There's really not much from toolbars.css that toolbox needs. It just needs -moz-appearance: toolbox; which may not require everything bug 1420229 is doing.
(In reply to Tim Nguyen :ntim from comment #2)
> (In reply to Brian Grinstead [:bgrins] from comment #1)
> > I'm assuming this also requires loading toolbars.css as a UA sheet so that
> > we don't lose those styles from toolbar-base
> 
> There's really not much from toolbars.css that toolbox needs. It just needs
> -moz-appearance: toolbox; which may not require everything bug 1420229 is
> doing.

Indeed - it looks like we could move the `-moz-appearance: toolbox` rule into xul.css since that's the only relevant thing set in toolbars.css (https://searchfox.org/mozilla-central/search?q=toolbox+%7B&path=).
Flags: needinfo?(timdream)
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
Comment hidden (mozreview-request)

Comment 5

Last year
mozreview-review
Comment on attachment 8948320 [details]
Bug 1429464 - Remove toolbox binding

https://reviewboard.mozilla.org/r/217802/#review223672

Thanks! 

Note that the try push is broken because of the cert stuff that's closing trees at the moment, but it loooooks like we might be OK here.

::: commit-message-ef1fe:4
(Diff revision 1)
> +- Remove the markup that initialize the palette field to null,
> +  effectively initialize the value to undefined, which is still
> +  falsey.

Can you add here that the toolbar binding remains responsible for initializing the toolbox's palette property with an actual DOM node? Thanks!
Attachment #8948320 - Flags: review?(gijskruitbosch+bugs) → review+
Comment hidden (mozreview-request)
Reporter

Updated

Last year
No longer depends on: 1420229

Comment 8

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/df6d2c3ee671
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Depends on: 1436351
Reporter

Updated

21 days ago
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.