Closed Bug 1538983 Opened 6 years ago Closed 6 years ago

Convert button and button-menu bindings to a Custom Element

Categories

(Toolkit :: XUL Widgets, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: bgrins, Assigned: aswan)

References

Details

Attachments

(3 files, 1 obsolete file)

Splitting this out of Bug 1519577, since I think this can be tackled separately.

It would include these two bindings, plus button-base: https://bgrins.github.io/xbl-analysis/tree/#button. Then toolbarbutton bindings could be handled separately.

I think I'd implement the base as a JS class, then extend it with a single MozButton class which switches markup based on the type attribute.

A couple notes:

  1. We also have to worry about buttons which inject their own DOM that doesn't match one of observes|template|menupopup|panel|tooltip, in which case all of the anonymous content gets dropped
  2. I'm a bit worried about perf here - there are something like 50 buttons in browser.xul at startup and although XBL does construct the anonymous DOM eagerly, no JS references are made. We've been able to avoid requiring a callback that runs when layout happens so far, but this is the type of binding where something like that would be handy. Perhaps making initializeAttributeInheritance and DOM construction happen on the C++ side stored through CustomElementRegistry would help, but since there are cases where we want to customize the behavior (i.e. (1)) that might not solve our problem.
Blocks: 1538547

General outline of what needs to happen:

Assignee: nobody → aswan

(In reply to Brian Grinstead [:bgrins] from comment #0)

  1. We also have to worry about buttons which inject their own DOM that doesn't match one of observes|template|menupopup|panel|tooltip, in which case all of the anonymous content gets dropped

I don't think we ever use panel/tooltip/template/observes, we do use menupopup. Would be worth checking before trying to port that over.

ButtonMenu should be registered as a Customized Built-in (like <button is="button-menu">) - see https://bugzilla.mozilla.org/show_bug.cgi?id=1519486#c0 for more details on how we did that with richlistitem.

We could combine Button/ButtonMenu into one single CE to avoid re-writing all consumers to use is="button-menu". That CE could check the type attribute.

Priority: -- → P3

(In reply to Tim Nguyen :ntim from comment #2)

(In reply to Brian Grinstead [:bgrins] from comment #0)

  1. We also have to worry about buttons which inject their own DOM that doesn't match one of observes|template|menupopup|panel|tooltip, in which case all of the anonymous content gets dropped

I don't think we ever use panel/tooltip/template/observes, we do use menupopup. Would be worth checking before trying to port that over.

there's a panel inside a button, see https://searchfox.org/mozilla-central/source/browser/components/customizableui/content/customizeMode.inc.xul#67, however it seems it should be a menupopup.

wip

Depends on: 1528268
Blocks: 1546352
Attachment #9058666 - Attachment description: Bug 1538983 Convert button xbl binding to a custom element → Bug 1538983 Convert button xbl binding to a custom element r=bgrins

Our standard <button> binding has been carrying around an extra <label>
just to support search highlighting in about:preferences. This patch
removes that overhead by moving the tweaks that about:preferences needs
into a custom element used only within about:preferences.

No longer blocks: 1546352
Attachment #9058667 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
No longer blocks: 1538547
Regressions: 1547326
Regressions: 1547377
Regressions: 1547463
Regressions: 1547767
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: