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

RESOLVED FIXED in Thunderbird 69.0

Status

task
RESOLVED FIXED
4 months ago
12 days ago

People

(Reporter: mkmelin, Assigned: mkmelin)

Tracking

(Blocks 1 bug)

Trunk
Thunderbird 69.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

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.

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)
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-

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: 3 months 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.