1535955 - Outgoing and incoming text encoding menulists in fonts dialog are empty

Status: RESOLVED FIXED

(Thunderbird :: Preferences, defect)

Description

[Richard Marti (:Paenglab)]

Since bug 1524508 the menulists for the outgoing and incoming text encoding of the fonts dialog are empty. Clicking on them and showing/selecting a charset works but nothing is shown in the menulists.

I see no error in the console.

Comment 1

[Magnus Melin [:mkmelin]]

Yes it is/was known. Should be same thing as for the calendar picker event dialogs.

Comment 2

[Jorg K (CEST = GMT+2)]

Umm, and we're shipping this broken in TB 67? Do I need to start a tracking flag for 67 now :-(

Comment 3

[Magnus Melin [:mkmelin]]

Attachment: bug1535955_menulist_selected_not_showing.patch

Comment 4

[Magnus Melin [:mkmelin]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4a058908d612c7a03062e5c0ddf8f875d0e17dc4

Comment 5

[Magnus Melin [:mkmelin]]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a91d2f87eec2d1b538b3ff87f86ba7023c43dbaa

Comment 6

[Khushil Mistry [:khushil324]]

Comment on attachment 9054141 [details] [diff] [review]
bug1535955_menulist_selected_not_showing.patch

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

Looks good to me. r=khushil. Just few changes related to code style.

::: mailnews/base/content/menulist-charsetpicker.js
@@ +11,1 @@
>     * - Setting preference="<name>" will set the selected value to that of the named preference value

This line is exceeding 80 characters.

@@ +89,5 @@
> +        "ISO-8859-1", "ISO-8859-7", "windows-1252",
> +      ];
> +    }
> +  }
> +  customElements.define("menulist-charsetpicker-sending", MozMenulistCharsetpickerSending, { extends: "menulist" });

This line is exceeding 80 characters.

@@ +106,5 @@
> +        "windows-1257", "windows-1258",
> +      ];
> +    }
> +  }
> +  customElements.define("menulist-charsetpicker-viewing", MozMenulistCharsetpickerViewing, { extends: "menulist" });

This line is exceeding 80 characters.

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1535955_menulist_selected_not_showing.patch

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9054680 [details] [diff] [review]
bug1535955_menulist_selected_not_showing.patch

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

::: mailnews/base/content/menulist-charsetpicker.js
@@ +86,5 @@
> +  class MozMenulistCharsetpickerSending extends MozMenulistCharsetpickerBase {
> +    get charsetValues() {
> +      return [
> +        "UTF-8", "EUC-KR", "gbk", "gb18030", "ISO-2022-JP",
> +        "ISO-8859-1", "ISO-8859-7", "windows-1252",

Time to remove "iso-8859-1" since it's just a label for "windows-1252", see here:
https://encoding.spec.whatwg.org/

@@ +102,5 @@
> +  class MozMenulistCharsetpickerViewing extends MozMenulistCharsetpickerBase {
> +    get charsetValues() {
> +      return [
> +        "UTF-8", "Big5", "EUC-KR", "gbk", "ISO-2022-JP",
> +        "ISO-8859-1", "ISO-8859-2", "ISO-8859-7",

And here.

Comment 9

[Jorg K (CEST = GMT+2)]

Well, sadly that will have to happen in a new bug since we still have:
mail/locales/en-US/chrome/messenger/messenger.properties
274 mailnews.view_default_charset=ISO-8859-1

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 9054680 [details] [diff] [review]
bug1535955_menulist_selected_not_showing.patch

Sadly this doesn't work. The selected items are now UTF-8 and ISO-8859-1 and choosing something else is just ignored. I'm surprised this passed review.

Comment 11

[Magnus Melin [:mkmelin]]

ISO-8859-1 was kept for a reason back when this was implemented. (To allow people to choose a the familiar charset.)

Comment 12

[Magnus Melin [:mkmelin]]

Attachment: bug1535955_menulist_selected_not_showing.patch

There was a bug for those wigets with preference attribute that snuck in. I'd tested mostly with the widget in folder properties which doesn't have that.

Comment 13

[Jorg K (CEST = GMT+2)]

Comment on attachment 9054710 [details] [diff] [review]
bug1535955_menulist_selected_not_showing.patch

Thanks, this works now. I'll land it tonight.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fe0a8fcab250
fix display of current selection of charset-picker menulists. r=khushil,jorgk DONTBUILD

Comment 15

[Jorg K (CEST = GMT+2)]

TB 67 beta:
https://hg.mozilla.org/releases/comm-beta/rev/d9af5609d45e9208eada712d2b210d35db2e45d6