866498 - Radio groups for switching {View > Feed Message Body As > Original HTML|Simple HTML|Plain Text} are not working and not synchronized between Main Menu and AppMenu (fix: use mailnews.display.html_as instead of rss.display.html_as until bug 458606 is fixed)

Status: VERIFIED FIXED

(Thunderbird :: General, defect)

Description

[sakshi]

Steps to Reproduce:

1) Select a "Blogs & News-Feeds" type account in folder list
2) Go to Main menu bar-> View-> Feed Message Body As->... (select any option e.g: Original HTML).
3) Go to App menu button [ ≡ ]-> view-> Feed Message Body As- > ...

Actual Result:

1) Main menu bar > View > Feed Message Body As >  ...: Radio checkmark is on the latest selected option (current status)
2) App menu button [ ≡ ] > View > Feed Message Body As >...: Radio checkmark is on another option (different status)

The Radio checkmark is on different options for the two menu items, i.e, the two radio groups are not in sync.

Expected result:
2) Main menu bar > View > Feed Message Body As >  ...: Radio checkmark is on the latest selected option
 (current status)
3) App menu button [ ≡ ] > View > Feed Message Body As >...: Radio checkmark is on the same option (current status)

The Radio checkmark must be on the current option for both the menu items, i.e, the two radio groups must always be in sync.

Comment 1

[Richard Marti (:Paenglab)]

On which TB version are you seeing this? I think this is a dupe of bug 793838 which is solved in TB 18.

Comment 2

[sakshi]

I guess mine is the older version 17.05.

Comment 3

[sakshi]

Is the bug bug 793838 fixing problem of keeping the Radio groups in sync for Original Html, Plain Html and Simple Text for Main Menu bar and App menu bar as well?

Comment 4

[Richard Marti (:Paenglab)]

Yes, it is. Maybe you can test the beta version to check it.

I mark this bug as dupe.

Comment 5

[Thomas D. (:thomas8)]

(In reply to sakshi from comment #3)
> Is the bug bug 793838 fixing problem of keeping the Radio groups in sync for
> Original Html, Plain Html and Simple Text for Main Menu bar and App menu bar
> as well?

No, radio checkmark synchronization does not work for all cases (but also the commands don't work...):

TB 23.0a1 (2013-04-28)

STR

1 View > Feed message body as > Summary (between Web page, Summary, and

Comment 6

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #5) - Sorry, premature release of comment ;)
(In reply to sakshi from comment #3)
> Is the bug bug 793838 fixing problem of keeping the Radio groups in sync for
> Original Html, Plain Html and Simple Text for Main Menu bar and App menu bar
> as well?

No, radio checkmark synchronization does not work for all cases (but also
the commands don't work...):

TB 23.0a1 (2013-04-28)

STR

1 View > Feed message body as > Summary (for the radio group of Web page, Summary, and Default format, the synchronization works both ways between the two menus)
2 Appmenu [!] > View > Feed message body as > Simple HTML (click)
3 check radio status in
  Main menu > View > Feed message body as > ...

Actual result
2 nothing happens (another bug, similar to Bug 55079)
3 on main menu, radio checkmark is still on Original HTML (so even though the command as such does nothing, it's not in sync with app menu)

Expected result
2 changing HTML/simple/plain flavors should work, or otherwise flavors should be disabled (bug 55079?)
3 if flavors are intended to work, then radio groups should be in sync

Note that you'll only see this when you start from appmenu, the flavors will be in sync if you toggle summary flavors from main menu.

I think we have to find out how exactly this menu is expected to work, which probably needs fixing bug 55079, and then revisit this to see if all radio groups are in sync no matter where you start.

Comment 7

[Thomas D. (:thomas8)]

(In reply to Thomas D. from comment #6)
> Actual result
> 2 nothing happens (another bug, similar to Bug 55079)

Sorry for the typo, that's Bug 550794 for all references in my above comment.

Comment 8

[Richard Marti (:Paenglab)]

For me the Web Page/Summary/Default Format are all the time synchronized. But it's true making a change in AppMenu on Original HTML/Simple HTML/Plain Text isn't synchronized with main menu.

And I see when 'Original HTML' is set and I change through main menu to 'Plain Text', the AppMenu shows 'Plain Text' but the main menu is still on 'Original HTML'. I can't say if changing this in AppMenu will change the view as in summary mode my feeds are looking in all three states the same (only a short text summary).

Comment 9

[Richard Marti (:Paenglab)]

Attachment: proposed fix

Okay, I found the issue. The menuitems changed the not implemented rss.display... prefs but the main menu checked the mailnews.display... prefs. The AppMenu menuitems set and checked the rss prefs and showed the correct state of this prefs. But this prefs had no effect in the visual appearance.

This patch corrects this and synchronizes the two functions InitViewBodyMenu() and InitAppmenuViewBodyMenu(). InitViewBodyMenu() was changed in a later bug but not InitAppmenuViewBodyMenu().

I removed also the try/catch as bug 650170 comment 75 suggested in InitAppmenuViewBodyMenu().

Comment 10

[Magnus Melin [:mkmelin]]

Comment on attachment 742829 [details] [diff] [review]
proposed fix

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

::: mail/base/content/mailWindowOverlay.js
@@ +586,5 @@
>                    "bodyFeedSummarySanitized",
>                    "bodyFeedSummaryAsPlaintext"];
>    var menuIDs = isFeed ? rssIDs : defaultIDs;
> +
> +  prefer_plaintext = Services.prefs.getBoolPref("mailnews.display.prefer_plaintext");

prefer_plaintext and others declarations can move here i think. (And make it let instead of var while you're at it.)

Comment 11

[Richard Marti (:Paenglab)]

Do you mean with move to change

let html_as = 0;
.
.
.
html_as = Services.prefs.getIntPref("mailnews.display.html_as");

to

let html_as = Services.prefs.getIntPref("mailnews.display.html_as");

etc.?

Comment 12

[Magnus Melin [:mkmelin]]

Yes.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: Patch for check-in: Use mailnews.display.html_as instead of rss.display.html_as until fix for bug 458606

Patch addressing the review comment.

Carrying over r+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/1dd4af110c7b

Comment 15

[Thomas D. (:thomas8)]

Verified fixed on WinXP/TB23.0a1 (2013-05-01)

Tweaking summary for streamlined readability and future tracking