Outgoing and incoming text encoding menulists in fonts dialog are empty

RESOLVED FIXED in Thunderbird 68.0

Status

defect
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: Paenglab, Assigned: mkmelin)

Tracking

({regression})

unspecified
Thunderbird 68.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird67 fixed, thunderbird68 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Reporter

Description

3 months ago

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.

Assignee

Comment 1

3 months ago

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

Comment 2

3 months ago

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

Assignee

Updated

3 months ago
Assignee: nobody → mkmelin+mozilla
Assignee

Comment 3

3 months ago
Attachment #9054141 - Flags: review?(khushil324)
Assignee

Updated

3 months ago
Status: NEW → ASSIGNED
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.
Attachment #9054141 - Flags: review?(khushil324) → review+
Assignee

Comment 7

3 months ago
Attachment #9054141 - Attachment is obsolete: true
Attachment #9054680 - Flags: review+
Assignee

Updated

3 months ago
Keywords: checkin-needed

Updated

3 months ago
Attachment #9054680 - Flags: approval-comm-beta+

Comment 8

3 months ago
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

3 months ago

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

Updated

3 months ago
Keywords: checkin-needed

Comment 10

3 months ago
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.
Flags: needinfo?(mkmelin+mozilla)
Attachment #9054680 - Flags: approval-comm-beta+ → review-
Assignee

Comment 11

3 months ago

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

Flags: needinfo?(mkmelin+mozilla)
Assignee

Comment 12

3 months ago

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.

Attachment #9054680 - Attachment is obsolete: true
Attachment #9054710 - Flags: review?(jorgk)

Comment 13

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

Thanks, this works now. I'll land it tonight.
Attachment #9054710 - Flags: review?(jorgk)
Attachment #9054710 - Flags: review+
Attachment #9054710 - Flags: approval-comm-beta+

Comment 14

3 months ago

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

Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED

Updated

3 months ago
Target Milestone: --- → Thunderbird 68.0
You need to log in before you can comment on or make changes to this bug.