Last Comment Bug 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)
: Radio groups for switching {View > Feed Message Body As > Original HTML|Simpl...
Status: VERIFIED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: unspecified
: x86 Linux
: -- minor (vote)
: Thunderbird 23.0
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on: 458606
Blocks: TB2SM 550794 793838
  Show dependency treegraph
 
Reported: 2013-04-28 02:52 PDT by sakshi
Modified: 2013-05-04 02:31 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed fix (5.67 KB, patch)
2013-04-28 07:57 PDT, Richard Marti (:Paenglab)
mkmelin+mozilla: review+
Details | Diff | Splinter Review
Patch for check-in: Use mailnews.display.html_as instead of rss.display.html_as until fix for bug 458606 (7.08 KB, patch)
2013-04-29 12:16 PDT, Richard Marti (:Paenglab)
richard.marti: review+
Details | Diff | Splinter Review

Description sakshi 2013-04-28 02:52:02 PDT
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) 2013-04-28 03:51:46 PDT
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 2013-04-28 04:02:56 PDT
I guess mine is the older version 17.05.
Comment 3 sakshi 2013-04-28 04:09:17 PDT
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) 2013-04-28 05:13:51 PDT
Yes, it is. Maybe you can test the beta version to check it.

I mark this bug as dupe.

*** This bug has been marked as a duplicate of bug 793838 ***
Comment 5 Thomas D. (currently busy elsewhere; needinfo?me) 2013-04-28 05:44:35 PDT
(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. (currently busy elsewhere; needinfo?me) 2013-04-28 05:56:53 PDT
(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. (currently busy elsewhere; needinfo?me) 2013-04-28 06:07:23 PDT
(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) 2013-04-28 06:32:12 PDT
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) 2013-04-28 07:57:33 PDT
Created attachment 742829 [details] [diff] [review]
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 2013-04-28 11:50:10 PDT
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) 2013-04-28 12:07:35 PDT
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 2013-04-29 11:20:25 PDT
Yes.
Comment 13 Richard Marti (:Paenglab) 2013-04-29 12:16:31 PDT
Created attachment 743213 [details] [diff] [review]
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] 2013-04-29 13:59:34 PDT
https://hg.mozilla.org/comm-central/rev/1dd4af110c7b
Comment 15 Thomas D. (currently busy elsewhere; needinfo?me) 2013-05-04 01:57:04 PDT
Verified fixed on WinXP/TB23.0a1 (2013-05-01)

Tweaking summary for streamlined readability and future tracking

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