Closed Bug 1535955 Opened 6 years ago Closed 6 years ago

Outgoing and incoming text encoding menulists in fonts dialog are empty

Categories

(Thunderbird :: Preferences, defect)

Product:
Thunderbird
Component:
Preferences
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(thunderbird67 fixed, thunderbird68 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 68.0
Tracking Flags:
Tracking Status
thunderbird67 --- fixed
thunderbird68 --- fixed

People

(Reporter: Paenglab, Assigned: mkmelin)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

bug1535955_menulist_selected_not_showing.patch
(deleted), patch
jorgk-bmo
: review+
jorgk-bmo
: approval-comm-beta+
Details | Diff | Splinter Review

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.

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

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

status-thunderbird67: --- → affected
tracking-thunderbird67: --- → +
Assignee: nobody → mkmelin+mozilla
Attached patch bug1535955_menulist_selected_not_showing.patch (obsolete) (deleted) — — Splinter Review
Attachment #9054141 - Flags: review?(khushil324)
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+
Attached patch bug1535955_menulist_selected_not_showing.patch (obsolete) (deleted) — — Splinter Review
Attachment #9054141 - Attachment is obsolete: true
Attachment #9054680 - Flags: review+
Keywords: checkin-needed
Attachment #9054680 - Flags: approval-comm-beta+
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.

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

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)

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

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: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 68.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: