391004 - "Open Browser with Message-ID" should be removed from References' and Message-IDs' context menu for emails

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Steffen Wilberg]

Bug 62033 added references to news and email. It's always enabled for news. For Email, you have to set the hidden pref mailnews.headers.references to true and restart Thunderbird.

The context menu for email references displays "Open Browser with Message-ID", which opens Google Groups. That doesn't make sense for emails since those usually don't show up there...
So this should be removed from the context menu for emails.

Comment 1

[Steffen Wilberg]

Same for Message-IDs, which can be enabled by setting mailnews.headers.showMessageId to true.

So we have
1. Email message header -> Message-ID context menu -> Open Browser with Message-ID
2. Email message header -> References context menu -> Open Browser with Message-ID
which should both be removed.

Comment 2

[Steffen Wilberg]

The pref for References mentioned in comment 0 is actually mailnews.headers.showReferences.

Comment 3

[Steffen Wilberg]

Attachment: screenshot of the context menu, and of Google Groups being unable to find my bugmail

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Comment 5

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

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

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

Aside from the comment below, r=me.

Thanks,
Blake.

::: mail/test/mozmill/message-header/test-message-header.js
@@ -213,0 +213,15 @@
> > +  * Ensure that the specified element is visible/hidden
> > +  *
> > +  * @param id the id of the element to check
> > +  * @param visible true if the element should be visible, false otherwise
NaN more ...

We don't have any news tests, do we?  Cause it would be nice to make sure that it does show up if we are on a News message…

Comment 6

[[e-mail]]

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

>+  // We don't want to show "Open Message For ID" for the same message
>+  // we're viewing.
Out of interest, how can this happen? We don't even know what the message ID will be until after the message has been created...

>+  var currentMsgId = "<" + gFolderDisplay.selectedMessage.messageId + ">";
>+  document.getElementById("messageIdContext-openMessageForMsgId")
>+          .setAttribute("hidden", (currentMsgId == msgId));
>+
>+  // We don't want to show "Open Browser With Message-ID" for non-nntp messages.
>+  document.getElementById("messageIdContext-openBrowserWithMsgId")
>+          .setAttribute("hidden", !gFolderDisplay.selectedMessageIsNews);
Nit: prefer the hidden property to the hidden attribute, even for menuitems.

Comment 7

[[e-mail]]

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

>+          .setAttribute("hidden", (currentMsgId == msgId));
Oh and the ()s can go too.

Comment 8

[[e-mail]]

(In reply to comment #6)
>(From update of attachment 583239 [details] [diff] [review])
>>+  // We don't want to show "Open Message For ID" for the same message
>>+  // we're viewing.
>Out of interest, how can this happen? We don't even know what the message ID
>will be until after the message has been created...
Oh I see, the same context menu is used for the message-id header too.

Comment 9

[Magnus Melin [:mkmelin]]

(In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #5)
> ::: mail/test/mozmill/message-header/test-message-header.js
> @@ -213,0 +213,15 @@
> > > +  * Ensure that the specified element is visible/hidden
> > > +  *
> > > +  * @param id the id of the element to check
> > > +  * @param visible true if the element should be visible, false otherwise
> NaN more ...

Sorry, i don't understand this comment?

> We don't have any news tests, do we?  Cause it would be nice to make sure
> that it does show up if we are on a News message…

Not that i know of, no...

Comment 10

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

(In reply to Magnus Melin from comment #9)
> (In reply to Blake Winton (:bwinton - Thunderbird UX) from comment #5)
> > ::: mail/test/mozmill/message-header/test-message-header.js
> > @@ -213,0 +213,15 @@
> > > > +  * Ensure that the specified element is visible/hidden
> > > > +  *
> > > > +  * @param id the id of the element to check
> > > > +  * @param visible true if the element should be visible, false otherwise
> > NaN more ...
> 
> Sorry, i don't understand this comment?

Yeah, that's the fancy new Splinter Review messing up like it always does.  :P

Feel free to ignore it.

> > We don't have any news tests, do we?  Cause it would be nice to make sure
> > that it does show up if we are on a News message…
> Not that i know of, no...

Pity.  Ah well.

Comment 11

[Magnus Melin [:mkmelin]]

http://hg.mozilla.org/comm-central/rev/c1d0f7844251
->FIXED

Comment 12

[Pascal Volk]

I just stumbled over this report.
The context menu entry 'Open Browser with Message-ID' works just fine for me - but mostly not with Google.

I've just set `mailnews.messageid_browser.url' to `http://mid.gmane.org/%mid'.

So, I guess there is no need to change anything.

Regards,
Pascal

Comment 13

[:aceman]

The item was already hidden by the merged patch. But maybe show it again if mailnews.messageid_browser.url is set to a non-default value?

Comment 14

[Magnus Melin [:mkmelin]]

I didn't realize there were such a use case.
Anyway, things should work for the large majority of users, not only the few users finding out a way to make it work for them. For the mentioned use case it's not likely working for all cases (someone using it for mail and for news). What it does should probably be more granual, perhaps per server or different mail vs news.