599036 - Feeds that should open in a new window opens in a new tab.

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Jørgen Rasmussen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; da; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 GTB7.1 (.NET CLR 2.0.50727; .NET CLR 3.0.4506.2152)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; da; rv:1.9.2.9) Gecko/20100915 Thunderbi

When I have selected a feed message and double click on it, the message opens in a new tab when it (perhaps) should open in a new window.
OR
The text in the menu items are wrong. Menu: Message -> Open Feed Message -> Web Page in New Window. 
AND  
Menu: Message -> Open Feed Message ->Summary in New Window.
Perhaps these strings are from before Thunderbird got tabs. Then "Window" should be changed to "Tab".

The strings are in messenger.dtd
<!ENTITY openFeedWebPageInWindow.label "Web Page in New Window">
<!ENTITY openFeedSummaryInWindow.label "Summary in New Window">

Reproducible: Always

Steps to Reproduce:
1.Select a feed message
2.From Message menu select "Open Feed Message" -> "Web Page in New Window" OR "Summary in New Window".
3.Double click om the selected feed message.
Actual Results:  
The feed message opens in a new tab.

Expected Results:  
It should open in a new window or the text should be changed to match what actual happens.

Personally I think the menu item texts are wrong.

Comment 1

[Ludovic Hirlimann [:Usul]]

What are your settings for opening a new message Tools -> Options -> Adanced -> reading and display ?

Comment 2

[Jørgen Rasmussen]

(In reply to comment #1)
> What are your settings for opening a new message Tools -> Options -> Adanced ->
> reading and display ?

I have checked the "Open message in a new tab".

I think there is a design problem here. I seems, that the easier accessible settings in the Message menu are subordinate to, and overruled by, the settings in the settings window.
If this is the intended behavior, shouldn't the menu items "Web Page in New Window" and "Summary in New Window" be disabled when "Open message in a new tab" is selected in the settings window? With this setting, choosing "Web Page in New Window" or "Summary in New Window" has no affect on how the feed messages are opened.

Comment 3

[Wayne Mery (:wsmwk)]

Reporter, do you still see this problem when using version 10 or newer?

if you are unable to reproduce, please close by setting status to resolved, and resolution to WORKSFORME or another appropriate setting.

If you are able to reproduce, please add new details, and a testcase if one does not already exist in the bug report.

Comment 4

[Jørgen Rasmussen]

(In reply to Wayne Mery (:wsmwk) from comment #3)
> Reporter, do you still see this problem when using version 10 or newer?
> 
> if you are unable to reproduce, please close by setting status to resolved,
> and resolution to WORKSFORME or another appropriate setting.
> 
> If you are able to reproduce, please add new details, and a testcase if one
> does not already exist in the bug report.

The problem is still there.

Comment 5

[alta88]

Attachment: patch

feeds came before tabs, and there was just window.  update the string for the possibility of either pref being set in Advanced -> Reading & Display.

(a more elegant detection of the pref and tailoring the string requires code in 2 places and is not worth it).

Comment 6

[alta88]

Attachment: patch

unchanged labels didn't cause l10n issues last time..

Comment 7

[Mark Banner (:standard8)]

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

Sorry for the delay in response.

To be honest, I think we've got too menu "how to open feed" options split in too many places. I never knew about this option until I looked into this bug.

And now I've changed the option that's in the Message window, that seems to totally override what's in the view menu, even when I change the view menu.

Whilst I can see the desire for flexibility, it feels like the options we've got are too complex.

That said, back to this patch, I don't think adding "or Tab" is the right way to go. Personally, I would rather see a preference such as "When opening feed messages open as...": "web page", "summary".

That way it doesn't matter what the user has selected elsewhere and would avoid the potential confusion.

Please also get ui-review on the changes before re-requesting review, you'll also need to change the string ids and the accesskeys to match as you are changing the context of the strings.

Comment 8

[alta88]

the choice of web page/summary is decided per folder in subscribe dialog, or globally overriden in View, and is not relevant for this bug.

a mail message open action in Message menu is set in Options, which pref is tab/new window/old window, plus 2 contextmenu items tab or window.  feeds additionally have have an open action to toggle in the message pane.

therefore, there should be a radio in Message for feeds, to either toggle, or open (window or tab like for mail).  however, there are the existing actions which users may already be used to.

so this language fix is merely to not lie about opening a feed in a window when the open action pref is set to tab.  not about a more radical change from the existing.

someone needs to explain exactly what an r+ fix is, i don't understand the suggestion or how it makes sense here.

Comment 9

[Mark Banner (:standard8)]

(In reply to alta88 from comment #8)
> someone needs to explain exactly what an r+ fix is, i don't understand the
> suggestion or how it makes sense here.

For me, ui-review+ depends on having a string that doesn't actually confuse the user even more. Saying "... Window or Tab" implies to me that the software doesn't actually know, and that doesn't feel right.

Comment 10

[alta88]

although you don't explicitly state it, i assume you mean doing what i thought wasn't worth it in comment 5.  and 'confuse the user' is pretty subjective no?

anyway, i belabor this because it is very unproductive to have a review methodology where someone submits a patch, and a reviewer nixes it with vague comments/questions or hasn't had time to delve into it and offers inapplicable suggestions, thus leading to a possibly endless patch/no/patch/no cycle.  no one is going to stick around long for that.  just state the exact thing needed for r+.  if you don't then you have defaulted the decision to the submittor, and it is then unfair to nitpick it.

Comment 11

[Mark Banner (:standard8)]

(In reply to alta88 from comment #10)
> although you don't explicitly state it, i assume you mean doing what i
> thought wasn't worth it in comment 5.  and 'confuse the user' is pretty
> subjective no?

Comment 5 is fine - I agree with the idea. It is the proposed end result I disagree with. 

> anyway, i belabor this because it is very unproductive to have a review
> methodology where someone submits a patch, and a reviewer nixes it with
> vague comments/questions or hasn't had time to delve into it and offers
> inapplicable suggestions, thus leading to a possibly endless
> patch/no/patch/no cycle.

As you should already know, I am not a ui-reviewer and I'm certainly not a UX expert, which is why this needs UX input. I did think I had redirected it for ui-review, however it appears I didn't, so if you think this is fine despite my comments, then you're welcome to request ui-review on it (I thought I already had redirected it to be honest).

> just state the exact thing needed for r+.

I stated this at the bottom of comment 7: ui-review+ and any other appropriate changes coming out of that or as requested (e.g. string ids). 

> if you don't then you have
> defaulted the decision to the submittor, and it is then unfair to nitpick it.

I was trying to express my opinion that I think the patch as-is is wrong from the user's perspective and the reasons why it is wrong. However, I don't have an opinion for what is better, except to redo the whole selection mechanism of how a user selects to have feeds being opened as summary or web page.

If ui-reviewers disagree with me, that's fine, and I'm happy to go with their decision.

Comment 12

[alta88]

i certainly do not disagree that revisiting/enhancing the (multitude of, given the nature of the beast) ways feeds can be opened would be useful.  for example, there is a hidden option to open the web page in a browser, very useful for power users and efficient processing of many feeds.  it's hidden because exposing UI options is not favored and i gave up. imo though, this bug is merely a language tweak.

bwinton, please indicate how to proceed, UI wise.

Comment 13

[Blake Winton (:bwinton) (:☕️)]

So, I'm not sure how much work it would be to switch the labels to say the right thing, but I'm going to trust you when you say that it wouldn't be worth it.

If we want to restrict this bug to a small string change (instead of using it as a chance to clean up the whole selection mechanism ;) then I think we could use "Open Feed Message » As Web Page" and "Open Feed Message » As Summary", and let people intuit that it will open in the same manner as "Open Message" opens (or differently if they've changed it)…  ("Open Feed Message » Toggle Web Page and Summary in Message Pane" is still an abomination, but I can't think of anything reasonable to change that to. :P  )

Thanks,
Blake.

Comment 14

[alta88]

Attachment: patch

as per suggestion.

Comment 15

[alta88]

Attachment: patch

patched patch.

Comment 16

[Blake Winton (:bwinton) (:☕️)]

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

>+++ b/mail/locales/en-US/chrome/messenger/messenger.dtd
>@@ -425,20 +425,20 @@
> <!ENTITY openConversationCmd.label "Open in Conversation">
> <!ENTITY openConversationCmd.accesskey "s">
> <!ENTITY openConversationCmd.key "o">
> <!ENTITY openFeedMessage.label "Open Feed Message">
> <!ENTITY openFeedMessage.accesskey "O">
>-<!ENTITY openFeedWebPageInWindow.label "Web Page in New Window">
>-<!ENTITY openFeedWebPageInWindow.accesskey "W">
>-<!ENTITY openFeedSummaryInWindow.label "Summary in New Window">
>-<!ENTITY openFeedSummaryInWindow.accesskey "S">
>+<!ENTITY openFeedWebPage.label "As Web Page">
>+<!ENTITY openFeedWebPage.accesskey "W">
>+<!ENTITY openFeedSummary.label "As Summary">
>+<!ENTITY openFeedSummary.accesskey "S">
> <!ENTITY openFeedWebPageInMP.label "Toggle Web Page and Summary in Message Pane">
> <!ENTITY openFeedWebPageInMP.accesskey "T">

So, I think I finally figured out why this looks so weird to me.
The menu item above it, "Open in Conversation", actually opens the message, whereas these just set an option.
What about "When Opening Feed Messages" followed by "Open As Web Page"/"Open As Summary"/"Toggle Web Page and Summary in Message Pane" ?

This is definitely an improvement over what we have, but I think we might be able to get it closer still.
Having said that, this has taken me so long to review, that I'm going to say r=me for this patch, and promise to review the next one in less time.

Thanks,
Blake.

Comment 17

[alta88]

Attachment: patch

yes, the new suggestion is better.  changing and carrying forward r+.

Comment 18

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/4d0b1de9de51