Closed
Bug 1297674
Opened 8 years ago
Closed 8 years ago
Menu item 'Copy' is inactive when viewing .eml attachment
Categories
(Thunderbird :: Message Reader UI, defect)
Tracking
(thunderbird48 wontfix, thunderbird49 fixed, thunderbird50 fixed, thunderbird_esr24 unaffected, thunderbird_esr38 unaffected, thunderbird_esr45 unaffected, thunderbird51 fixed)
RESOLVED
FIXED
Thunderbird 51.0
Tracking | Status | |
---|---|---|
thunderbird48 | --- | wontfix |
thunderbird49 | --- | fixed |
thunderbird50 | --- | fixed |
thunderbird_esr24 | --- | unaffected |
thunderbird_esr38 | --- | unaffected |
thunderbird_esr45 | --- | unaffected |
thunderbird51 | --- | fixed |
People
(Reporter: psnmbox, Assigned: jorgk-bmo)
References
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
jorgk-bmo
:
review+
jorgk-bmo
:
approval-comm-aurora+
jorgk-bmo
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160823072522
Steps to reproduce:
Windows 10 32-bit.
I received copy of an e-mail as attached .eml file.
Clicked on it and the attachment was opened in another window.
Then selected some text in the window (could be done with keyboard or mouse).
Actual results:
In context menu items Copy and Select All are always disabled even though corresponding keyboard shortcuts are active.
Expected results:
The message is in read-only state, but context menu items Copy and Select All should be active.
Assignee | ||
Comment 1•8 years ago
|
||
Hmm, in my TB 51 Daily the context (right-click) menu looks pretty terrible on a message opened from a file, there are even empty entries. So something has gone bust there.
|Copy/Select all| are disabled. This worked OK in TB 45. Thanks for reporting this.
Aceman, can you have a look.
Status: UNCONFIRMED → NEW
Component: Untriaged → Message Reader UI
Ever confirmed: true
Flags: needinfo?(acelists)
Keywords: regression
Assignee | ||
Comment 2•8 years ago
|
||
Alice, if you have a lot of time, you could look for the regression here.
STR:
- open an .eml file using File > Open > Saved Message
(Menu varies in different versions).
- Right click on the message body and inspect the menu.
The menu should have about 10 entries, "Select All" at the top.
Right now, in TB 51, it has about 50+ entries all mixed up terribly.
TB 45 was OK, TB 51 is not.
Flags: needinfo?(alice0775)
Comment 3•8 years ago
|
||
Good: 20160308030405
Bad : 20160327030426
(No windows build between them)
Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=05c087337043dd8e71cc27bdb5b9d55fd00aaa26&tochange=63be002b4a803df1122823841ef7633b7561d873
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=3e342c12aed2&tochange=5a74c06663ea
Regressed by:
e2845f0b82eb Thomas Düllmann — Bug 1106412 - Implement Edit Draft Message Command. r=jorgk
Error console in the bad build:
Timestamp: 2016/08/25 10:38:25
Error: TypeError: msg.folder is null
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 575
Timestamp: 2016/08/25 10:38:27
Error: TypeError: gContextMenu is undefined
Source File: chrome://messenger/content/mailContextMenus.js
Line: 50
Blocks: 1106412
status-thunderbird48:
--- → wontfix
status-thunderbird49:
--- → affected
status-thunderbird50:
--- → affected
status-thunderbird51:
--- → affected
tracking-thunderbird49:
--- → ?
tracking-thunderbird50:
--- → ?
tracking-thunderbird51:
--- → ?
Flags: needinfo?(alice0775)
Assignee | ||
Comment 4•8 years ago
|
||
Alice, thank you so much!!!
Aceman, another one we broke, grrr.
Assignee | ||
Comment 5•8 years ago
|
||
Clearing the tracking flags. I will land on TB 51 and uplift to TB 50 and TB 49.
tracking-thunderbird49:
? → ---
tracking-thunderbird50:
? → ---
tracking-thunderbird51:
? → ---
Flags: needinfo?(acelists)
Assignee | ||
Comment 6•8 years ago
|
||
One review only, whoever comes first.
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Attachment #8784707 -
Flags: review?(mkmelin+mozilla)
Attachment #8784707 -
Flags: review?(acelists)
Comment 7•8 years ago
|
||
Comment on attachment 8784707 [details] [diff] [review]
Proposed fix (v1).
Review of attachment 8784707 [details] [diff] [review]:
-----------------------------------------------------------------
Works, but I'd simplify it like below.
::: mail/base/content/mailWindowOverlay.js
@@ +580,2 @@
> let folder = gFolderDisplay.displayedFolder;
> let inDraftFolder = (msg &&
You could just make this row
let inDraftFolder = (msg && msg.folder
Attachment #8784707 -
Flags: review?(mkmelin+mozilla)
Attachment #8784707 -
Flags: review?(acelists)
Attachment #8784707 -
Flags: review+
Comment 8•8 years ago
|
||
Well,
let inDraftFolder = (msg && msg.folder &&
Assignee | ||
Comment 9•8 years ago
|
||
Thanks, that was quick ;-)
Your approach is nicer. I left the comment in, I hope you don't mind.
Attachment #8784707 -
Attachment is obsolete: true
Attachment #8784969 -
Flags: review+
Assignee | ||
Comment 10•8 years ago
|
||
Easy candidate for those looking for something to land ;-)
Keywords: checkin-needed
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8784969 [details] [diff] [review]
Proposed fix (v1b).
[Approval Request Comment]
Regression caused by (bug #): Bug 1106412
User impact if declined: Right menu for messages opened from file doesn't work.
Testing completed (on c-c, etc.): Manual.
Risk to taking this patch (and alternatives if risky):
No risk.
Attachment #8784969 -
Flags: approval-comm-beta+
Attachment #8784969 -
Flags: approval-comm-aurora+
Comment 12•8 years ago
|
||
http://hg.mozilla.org/comm-central/rev/635867a05c38
http://hg.mozilla.org/releases/comm-aurora/rev/95c294c874f9
http://hg.mozilla.org/releases/comm-beta/rev/bf311c97af59
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 51.0
Assignee | ||
Comment 13•8 years ago
|
||
Thanks for pushing this. However, as already discussed in bug 507640 comment #60, I handle the uplifts personally. I do them in blocks in order not to push trivial changes individually. If you want this job, you can have it ;-) In this case I also wanted to push bug 1211005. Now I have to do this separately.
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to Jorg K (GMT+2, PTO during summer) from comment #13)
> Now I have to do this separately.
And I will land them with DONTBUILD since the test can run next time a build is done, most likely through a blocklist update on the weekend.
Comment 15•8 years ago
|
||
(In reply to Jorg K (GMT+2, PTO during summer) from comment #10)
> Easy candidate for those looking for something to land ;-)
In that case, you probably shouldn't have set the "checkin-needed" flag or provide some instructions where to land in the whiteboard. And no, I don't want to have that job. 8-)
Assignee | ||
Comment 16•8 years ago
|
||
My comment was mostly for the Aurora and beta uplifts that I usually handle myself.
Anyway, the push to C-C revealed a new test failure: Bug 1298339.
Comment 17•8 years ago
|
||
(In reply to Jorg K (GMT+2, PTO during summer) from comment #16)
> My comment was mostly for the Aurora and beta uplifts
I know, but it was ambiguous. I had usually put something like "[c-n: comm-central]" into the whiteboard with a checkin-needed to be specific where a patch has to land.
> Anyway, the push to C-C revealed a new test failure: Bug 1298339.
Well, at least something good came out of it...
Reporter | ||
Comment 18•8 years ago
|
||
Checked today's 32-bit update and this bug appears to be fixed.
Though while reading your e-mail exchange I see a different issue, possibly this one: https://bugzilla.mozilla.org/show_bug.cgi?id=199433
You need to log in
before you can comment on or make changes to this bug.
Description
•