Last Comment Bug 720420 - Context menu in Compose Window has non-functional and unrelated options
: Context menu in Compose Window has non-functional and unrelated options
Status: RESOLVED FIXED
: regression
Product: Thunderbird
Classification: Client Software
Component: Message Reader UI (show other bugs)
: 11 Branch
: All All
: -- normal (vote)
: Thunderbird 13.0
Assigned To: Jim Porter (:squib)
:
Mentors:
: 724320 (view as bug list)
Depends on:
Blocks: 680192
  Show dependency treegraph
 
Reported: 2012-01-23 10:43 PST by Mike Conley (:mconley) - (needinfo me!)
Modified: 2012-02-21 07:56 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
fixed


Attachments
A demonstration of the problem (166.63 KB, image/png)
2012-01-23 10:43 PST, Mike Conley (:mconley) - (needinfo me!)
no flags Details
Fix this (1.16 KB, patch)
2012-02-04 21:34 PST, Jim Porter (:squib)
mconley: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
Details | Diff | Review

Description Mike Conley (:mconley) - (needinfo me!) 2012-01-23 10:43:23 PST
Created attachment 590783 [details]
A demonstration of the problem

Steps to reproduce:

1)  Open up an email message in a new window (right click on the message, choose "Open Message in New Window")
2)  Right click on the message text

What happens?

A massive context menu comes up, with options for doing things that make no sense in the context of the message ("Play", "Mute", "Unmute", "Reply to Newsgroup", etc).

What's expected?

A smaller list should appear, with options that only make sense in the context of the message I'm viewing.

See attachment.
Comment 1 Jim Porter (:squib) 2012-01-23 10:59:53 PST
Hm, that's my fault. PageMenu.jsm isn't defined in the standalone message window. The various PageMenu bits[1] should probably be moved to nsContextMenu.js?

[1] http://mxr.mozilla.org/comm-central/search?string=PageMenu&find=mail%2Fbase%2F*&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
Comment 2 Jim Porter (:squib) 2012-01-23 11:20:20 PST
Note also that (I think) the way this is handled currently, HTML messages could theoretically add context menu items (but no Javascript code can execute). Maybe we should disallow that, since it's not very useful without JS...
Comment 3 Jim Porter (:squib) 2012-02-04 20:16:44 PST
*** Bug 724320 has been marked as a duplicate of this bug. ***
Comment 4 Jim Porter (:squib) 2012-02-04 21:34:04 PST
Created attachment 594509 [details] [diff] [review]
Fix this

This should fix things. Let me know if you want automated tests for it.
Comment 5 Nomis101 2012-02-05 01:21:25 PST
*** Bug 724326 has been marked as a duplicate of this bug. ***
Comment 6 Nomis101 2012-02-12 15:32:56 PST
(In reply to Jim Porter (:squib) from comment #4)
> This should fix things.
Yes, it does, I can confirm that this patch fixes all issues I have with right click menu on standalone window with Thunderbird 11. :-)
Comment 7 Mike Conley (:mconley) - (needinfo me!) 2012-02-12 15:35:38 PST
Hey Jim,

Work-week swamped me, and I couldn't get any reviews done.  I'll have this reviewed tomorrow.

-Mike
Comment 8 Mike Conley (:mconley) - (needinfo me!) 2012-02-13 08:47:16 PST
Comment on attachment 594509 [details] [diff] [review]
Fix this

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

This looks good to me.

If I could have one more thing, it'd be a Mozmill test for this, if you have the cycles.  If not, maybe file a bug for one.
Comment 9 Jim Porter (:squib) 2012-02-13 23:48:52 PST
I'd really rather figure out a way to make any JS error trigger a Mozmill failure. That probably would have caught this. (Otherwise, we run the risk of explicitly testing *everything*, which sounds nice until you think about what "everything" means...)
Comment 10 Mike Conley (:mconley) - (needinfo me!) 2012-02-14 07:17:54 PST
(In reply to Jim Porter (:squib) from comment #9)
> I'd really rather figure out a way to make any JS error trigger a Mozmill
> failure. That probably would have caught this. (Otherwise, we run the risk
> of explicitly testing *everything*, which sounds nice until you think about
> what "everything" means...)

Touché! :)
Comment 11 Jim Porter (:squib) 2012-02-14 23:45:13 PST
Checked in: http://hg.mozilla.org/comm-central/rev/ebd6300d9dfc
Comment 12 Jim Porter (:squib) 2012-02-14 23:46:47 PST
Comment on attachment 594509 [details] [diff] [review]
Fix this

We should definitely take this for beta and aurora, since this totally breaks the context menu in the standalone window.

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