793838 - Feed body views AppMenu items are not working anymore ( [ ≡ ] > View > Feed Message Body As > Web Page | Summary | Default Format)

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Richard Marti (:Paenglab)]

Bug 596234 changed on main menu items the oncommand from ChangeFeedShowSummaryPref() to FeedMessageHandler.onSelectPref = but forgot the AppMenu items. The AppMenu items are also not showing the checked mode.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: patch

This patch fixes the issues. But when I change the view with the AppMenu items the checked state on main menu isn't updated when this isn't opened before I change the view. It looks like the main menu items aren't initialized correctly.

Mike, do you have a solution for this problem?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Investigating...

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Paenglab:

Hm - I can't seem to reproduce the problem - the View > Feed Message Body As items in the main menu seem to change properly when I update them from the AppMenu.

If you can reproduce it reliably, can you give me detailed STR?

-Mike

Comment 4

[Richard Marti (:Paenglab)]

STR:
- Feed Message Body has to be set on Summary.
- Close TB.
- Open TB, select a feed message.
- Change in AppMenu the view from Summary to 'Web Page'.
- Open the main menu and go to View/'Feed Message Body'. Summary is still checked.
- Move in View menu to Headers and let open the submenu.
- Move back to 'Feed Message Body'. 'Web Page' is selected.

This also works from 'Default Format' to the two other options.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Excellent - thanks, reproduced.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1

Wow, that was a lot harder to figure out than I expected.

Basically, I think we've hit a XUL bug here. I think we're experiencing a bug with menuitem's autocheck feature when type="radio".

Unchecking the other menuitems seems to fix it. Let's roll with that.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 664200 [details] [diff] [review]
patch

See above.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: patch v2

Combination of mconley's and my patch.

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 664644 [details] [diff] [review]
patch v2

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

Thx Richard! r=mkmelin

::: mail/base/content/mailWindowOverlay.js
@@ +221,5 @@
>                              "bodyFeedPerFolderPref"];
>    let checked = FeedMessageHandler.onSelectPref;
> +  for each (let [index, id] in Iterator(viewRssMenuItemIds)) {
> +    document.getElementById(id)
> +            .setAttribute("checked", index == checked);

[i wonder if setting the checked property instead of the attribute would have fixed it?]

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/56461e6f48d3

Comment 12

[Thomas D. (:thomas8)]

View > Message Body As > ...

This bug fixed functionality of feed body view modes:
  Web Page | Summary | Default Format
The other half of the menu, functionality of HTML rendering pref, was fixed in bug 866498:
  Original HTML | Simple HTML | Plain Text

Setting dependency for ease of tracking.

Related Bug 550794 is about putting {Feed view modes} vs. {HTML rendering prefs} into the right order (proposed solution: Feed view modes first).