Closed Bug 391004 Opened 13 years ago Closed 8 years ago

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

Categories

(Thunderbird :: Mail Window Front End, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 12.0

People

(Reporter: steffen.wilberg, Assigned: mkmelin)

References

Details

Attachments

(2 files)

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.
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.
Summary: "Open Browser with Message-ID" should be removed from email references' context menu → "Open Browser with Message-ID" should be removed from References' and Message-IDs' context menu for emails
The pref for References mentioned in comment 0 is actually mailnews.headers.showReferences.
OS: Windows Vista → All
Hardware: PC → All
Attached patch proposed fixSplinter Review
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #583239 - Flags: superreview?(neil)
Attachment #583239 - Flags: review?(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…
Attachment #583239 - Flags: review?(bwinton) → review+
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.
Attachment #583239 - Flags: superreview?(neil) → superreview+
Comment on attachment 583239 [details] [diff] [review]
proposed fix

>+          .setAttribute("hidden", (currentMsgId == msgId));
Oh and the ()s can go too.
(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.
(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...
(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.
http://hg.mozilla.org/comm-central/rev/c1d0f7844251
->FIXED
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 12.0
Flags: in-testsuite+
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
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?
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.
You need to log in before you can comment on or make changes to this bug.