1548852 - Quickfix some parts (main menu and messengerLanguages.js) after bug 1500626

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

Especially Linux's main menu is broken after bug 1500626. Also messengerLanguages.js needs a fix that will be lost if we don't do it here.

The AppMenu stays broken because it misses other removed XBL parts.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1548852-quick-fix-after-1500626.patch

When you test, you have to remove menu.xml in your obj-dir because it was removed in bug 1500626.

This patch fixes the main menu on Linux and ports the change https://hg.mozilla.org/mozilla-central/rev/37d758a90ed9#l1.1 to messengerLanguages.js. I removed some references to chrome://global/content/bindings/menu.xml in the CSS files.

The AppMenu stays broken and needs de-XBL work.

Comment 2

[Jorg K (CEST = GMT+2)]

This must be damage Geoff mentioned in a PM.

Comment 3

[Jorg K (CEST = GMT+2)]

Comment on attachment 9062495 [details] [diff] [review]
1548852-quick-fix-after-1500626.patch

Review of attachment 9062495 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with comment/question.

::: mail/components/preferences/messengerLanguages.js
@@ +216,3 @@
>      item.setAttribute("label", label);
> +    if (value)
> +      item.value = value;

Why are we doing this? Don't we always want to assign the value passed in. Well, I guess no need to assign a null/undefined since the item creation will have pre-populated the value.

Comment 4

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #3)

Comment on attachment 9062495 [details] [diff] [review]
1548852-quick-fix-after-1500626.patch

Review of attachment 9062495 [details] [diff] [review]:

r+ with comment/question.

::: mail/components/preferences/messengerLanguages.js
@@ +216,3 @@

 item.setAttribute("label", label);

• if (value)

•  item.value = value;
  

Why are we doing this? Don't we always want to assign the value passed in.
Well, I guess no need to assign a null/undefined since the item creation
will have pre-populated the value.

You need to ask the m-c people. I only ported it.

Comment 5

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #1)

The AppMenu stays broken and needs de-XBL work.

Magnus, can you please file a bug for the follow-up work. I'm going to close this bug here once the patch has landed.

Strangely there are not test failures although we exercise the Hamburger menu a lot in MozMill testing.

Comment 6

[Magnus Melin [:mkmelin]]

The appmenu de-xbl is bug 1546309. There is more we should do though - I'll file those.

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9062495 [details] [diff] [review]
1548852-quick-fix-after-1500626.patch

Review of attachment 9062495 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/messenger.css
@@ -147,5 @@
>    -moz-image-region: inherit;
>  }
>  
> -.splitmenu-menuitem[iconic="true"] {
> -  -moz-binding: url("chrome://global/content/bindings/menu.xml#menuitem-iconic");

could you just comment out this instead, that will make it easier to see what to fix

::: mail/themes/linux/mail/messenger.css
@@ -544,5 @@
>  }
>  
> -/* Stock icons for the menu bar items */
> -menuitem:not([type]) {
> -  -moz-binding: url("chrome://global/content/bindings/menu.xml#menuitem-iconic");

this too, only comment it out

::: mail/themes/linux/mail/primaryToolbar.css
@@ -296,5 @@
>    list-style-image: url("chrome://messenger/skin/icons/search-glass.svg");
>  }
>  
> -#appmenu_find > .splitmenu-menuitem {
> -  -moz-binding: url("chrome://global/content/bindings/menu.xml#menuitem-iconic");

and this

Comment 8

[Richard Marti (:Paenglab)]

Attachment: 1548852-quick-fix-after-1500626.patch

Commented out instead of deleted.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cb59a5f58ca6
Remove use of menuitem* binding in CSS files after removal in bug 1500626. r=jorgk