Last Comment Bug 793838 - Feed body views AppMenu items are not working anymore ( [ ≡ ] > View > Feed Message Body As > Web Page | Summary | Default Format)
: Feed body views AppMenu items are not working anymore ( [ ≡ ] > View > Feed M...
Product: Thunderbird
Classification: Client Software
Component: Message Reader UI (show other bugs)
: unspecified
: All All
-- normal (vote)
: Thunderbird 18.0
Assigned To: Richard Marti (:Paenglab)
Depends on: 866498
Blocks: 787612
  Show dependency treegraph
Reported: 2012-09-24 13:52 PDT by Richard Marti (:Paenglab)
Modified: 2013-05-04 02:31 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (2.38 KB, patch)
2012-09-24 14:00 PDT, Richard Marti (:Paenglab)
mconley: feedback+
Details | Diff | Splinter Review
Patch v1 (1.15 KB, patch)
2012-09-25 13:32 PDT, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review
patch v2 (3.53 KB, patch)
2012-09-25 14:07 PDT, Richard Marti (:Paenglab)
mkmelin+mozilla: review+
Details | Diff | Splinter Review

Description User image Richard Marti (:Paenglab) 2012-09-24 13:52:45 PDT
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 User image Richard Marti (:Paenglab) 2012-09-24 14:00:30 PDT
Created attachment 664200 [details] [diff] [review]

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 User image Mike Conley (:mconley) 2012-09-25 08:46:57 PDT
Comment 3 User image Mike Conley (:mconley) 2012-09-25 11:18:16 PDT

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?

Comment 4 User image Richard Marti (:Paenglab) 2012-09-25 11:39:50 PDT
- 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 User image Mike Conley (:mconley) 2012-09-25 11:49:15 PDT
Excellent - thanks, reproduced.
Comment 6 User image Mike Conley (:mconley) 2012-09-25 13:32:40 PDT
Created attachment 664631 [details] [diff] [review]
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 User image Mike Conley (:mconley) 2012-09-25 13:32:53 PDT
Comment on attachment 664200 [details] [diff] [review]

See above.
Comment 8 User image Richard Marti (:Paenglab) 2012-09-25 14:07:14 PDT
Created attachment 664644 [details] [diff] [review]
patch v2

Combination of mconley's and my patch.
Comment 9 User image Magnus Melin 2012-09-28 12:34:09 PDT
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 User image Ryan VanderMeulen [:RyanVM] 2012-09-29 06:35:35 PDT
Comment 11 User image Richard Marti (:Paenglab) 2013-04-28 05:13:51 PDT
*** Bug 866498 has been marked as a duplicate of this bug. ***
Comment 12 User image Thomas D. (currently busy elsewhere; needinfo?me) 2013-05-04 02:31:43 PDT
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).

Note You need to log in before you can comment on or make changes to this bug.