Closed Bug 1547233 Opened 6 years ago Closed 5 years ago

[de-xbl] convert toolbar.xml#menu-button binding to custom element

Categories

(Thunderbird :: Toolbars and Tabs, task)

Product:
Thunderbird
Component:
Toolbars and Tabs
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 69.0

People

(Reporter: mkmelin, Assigned: mkmelin)

References

Details

Attachments

(1 file, 3 obsolete files)

bug1547233_toolbar_menu-button.patch
(deleted), patch
aleca
: review+
khushil324
: review+
Details | Diff | Splinter Review

Not to be confused with bug 1546352 (which is very related, but not the same)

Convert the toolbar.xml#menu-button binding to custom element.
https://searchfox.org/comm-central/rev/01f50ef57ff126a8a8146f9091a44d3ff0b31d4e/common/bindings/toolbar.xml#519

This binding inherits button-base which will be removed in bug 1519577.

Attached patch bug1547233_toolbar_menu-button.patch (obsolete) (deleted) — — Splinter Review

This seems to do it (with arc patch D31941 ).

One thing is missing in action: new Write button now longer has the lightning items. Lightning overlays this (button-newmsg) but button-newmsg is not a menu-button toolbarbutton to start with (in thunderbird core), "is" can't be set dynamically, and it can't be a toolbarbutton-menu-button initially when there is no child element... I experimented a bunch with changing the internals dynamically, but couldn't make that work properly.

Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #9066681 - Flags: review?(khushil324)
Attached patch bug1547233_toolbar_menu-button.patch (obsolete) (deleted) — — Splinter Review
Attachment #9066681 - Attachment is obsolete: true
Attachment #9066681 - Flags: review?(khushil324)
Attachment #9066682 - Flags: review?(khushil324)
Comment on attachment 9066682 [details] [diff] [review] bug1547233_toolbar_menu-button.patch Review of attachment 9066682 [details] [diff] [review]: ----------------------------------------------------------------- I am getting this error in console : TypeError: class heritage customElements.get(...) is not an object or null Also, label text is not vertically center aligned on Mac. I have attached the SS. I will look into it and update you. ::: mail/components/compose/content/messengercompose.xul @@ +1846,5 @@ > > + <toolbarbutton is="toolbarbutton-menu-button" id="spellingButton" > + type="menu-button" > + class="toolbarbutton-1" > + label="&spellingButton.label;" Indentation is off. @@ +1858,5 @@ > > + <toolbarbutton is="toolbarbutton-menu-button" id="button-save" > + type="menu-button" > + class="toolbarbutton-1" > + label="&saveButton.label;" Indentation is off.
Attachment #9066682 - Flags: review?(khushil324) → review-
Attached image Screenshot 2019-05-23 at 1.41.41 AM.png (obsolete) (deleted) —

I think you didn't you apply D31941 in m-c first?
If customElements.get(...) was null all the toolbarbutton-menu-button custom element will not be set up and that would cause the problem in the screen shot.

Fixed the misalignment. The patch works find for me on linux. Needs m-c arc patch D31941

Attachment #9066987 - Flags: review?(khushil324)
Attachment #9066987 - Flags: review?(alessandro)
Attachment #9066682 - Attachment is obsolete: true
Attachment #9066856 - Attachment is obsolete: true
Comment on attachment 9066987 [details] [diff] [review] bug1547233_toolbar_menu-button.patch Review of attachment 9066987 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. r=khushil. Just the following nit. ::: mail/base/content/toolbarbutton-menu-button.js @@ +61,5 @@ > + > + this.render(); > + } > + } > + customElements.define("toolbarbutton-menu-button", MozToolbarButtonMenuButton, { extends: "toolbarbutton" }); Extending the character limit. ::: mail/components/compose/content/messengercompose.xul @@ +1846,5 @@ > > + <toolbarbutton is="toolbarbutton-menu-button" id="spellingButton" > + type="menu-button" > + class="toolbarbutton-1" > + label="&spellingButton.label;" Indentation is off.
Attachment #9066987 - Flags: review?(khushil324) → review+

Yes, that's part of the try push, as I said "... and a few friends".

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/b7c1cd832766
[de-xbl] convert toolbar.xml#menu-button binding to custom element. r=khushil

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 69.0
Comment on attachment 9066987 [details] [diff] [review] bug1547233_toolbar_menu-button.patch Review of attachment 9066987 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me as well other than what khushil324 already reported.
Attachment #9066987 - Flags: review?(alessandro) → review+
Regressions: 1554300
Regressions: 1555608
Regressions: 1572379
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: