Last Comment Bug 391004 - "Open Browser with Message-ID" should be removed from References' and Message-IDs' context menu for emails
: "Open Browser with Message-ID" should be removed from References' and Message...
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: Thunderbird 12.0
Assigned To: Magnus Melin
:
Mentors:
Depends on:
Blocks: 62033
  Show dependency treegraph
 
Reported: 2007-08-05 07:26 PDT by Steffen Wilberg
Modified: 2012-01-14 10:50 PST (History)
8 users (show)
mkmelin+mozilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
screenshot of the context menu, and of Google Groups being unable to find my bugmail (17.01 KB, image/png)
2009-03-08 09:04 PDT, Steffen Wilberg
no flags Details
proposed fix (9.79 KB, patch)
2011-12-20 12:02 PST, Magnus Melin
bwinton: review+
neil: superreview+
Details | Diff | Splinter Review

Description Steffen Wilberg 2007-08-05 07:26:36 PDT
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 2007-08-05 07:36:41 PDT
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 2007-08-05 08:12:13 PDT
The pref for References mentioned in comment 0 is actually mailnews.headers.showReferences.
Comment 3 Steffen Wilberg 2009-03-08 09:04:42 PDT
Created attachment 366190 [details]
screenshot of the context menu, and of Google Groups being unable to find my bugmail
Comment 4 Magnus Melin 2011-12-20 12:02:12 PST
Created attachment 583239 [details] [diff] [review]
proposed fix
Comment 5 Blake Winton (:bwinton) (:☕️) 2011-12-20 13:16:15 PST
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 neil@parkwaycc.co.uk 2011-12-20 13:52:11 PST
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 neil@parkwaycc.co.uk 2011-12-20 13:53:16 PST
Comment on attachment 583239 [details] [diff] [review]
proposed fix

>+          .setAttribute("hidden", (currentMsgId == msgId));
Oh and the ()s can go too.
Comment 8 neil@parkwaycc.co.uk 2011-12-20 13:54:53 PST
(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 2011-12-22 11:10:50 PST
(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) (:☕️) 2011-12-22 11:22:12 PST
(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 2011-12-30 10:56:48 PST
http://hg.mozilla.org/comm-central/rev/c1d0f7844251
->FIXED
Comment 12 Pascal Volk 2011-12-30 11:21:21 PST
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 2012-01-09 08:20:38 PST
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 2012-01-14 10:50:56 PST
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.

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