Thunderbird: File attachments in attachments bucket in compose window don't open (breakage from bug 1203813)

RESOLVED FIXED in Thunderbird 52.0

Status

Thunderbird
Message Compose Window
RESOLVED FIXED
a year ago
7 months ago

People

(Reporter: Jorg K (GMT+2), Assigned: Jorg K (GMT+2))

Tracking

Trunk
Thunderbird 52.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (obsolete)
Comment hidden (obsolete)
(Assignee)

Comment 2

7 months ago
As of today, this failure shows on all platforms. Most likely it's a different failure, but since I didn't record any details for this failure here, we might as well use this bug.

Failing run:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2b410485bd8e

Log:
https://archive.mozilla.org/pub/thunderbird/try-builds/mozilla@jorgk.com-2b410485bd8e/try-comm-central-linux/try-comm-central_ubuntu32_vm_test-mozmill-bm08-tests1-linux32-build5.txt.gz

11:32:46     INFO -  SUMMARY-UNEXPECTED-FAIL | test-attachment.js | test-attachment.js::test_open_attachment
11:32:46     INFO -    EXCEPTION: Timeout waiting for modal dialog to open.
11:32:46     INFO -      at: utils.js line 447
11:32:46     INFO -         TimeoutError utils.js:447 13
11:32:46     INFO -         waitFor utils.js:485 11
11:32:46     INFO -         WindowWatcher_waitForModalDialog test-window-helpers.js:376 5
11:32:46     INFO -         wait_for_modal_dialog test-window-helpers.js:613 3
11:32:46     INFO -         test_open_attachment test-attachment.js:266 3
11:32:46     INFO -         Runner.prototype.wrapper frame.js:585 9
11:32:46     INFO -         Runner.prototype._runTestModule frame.js:655 9
11:32:46     INFO -         Runner.prototype.runTestModule frame.js:701 3
11:32:46     INFO -         Runner.prototype.runTestDirectory frame.js:525 7
11:32:46     INFO -         runTestDirectory frame.js:707 3
11:32:46     INFO -         Bridge.prototype._execFunction server.js:179 10
11:32:46     INFO -         Bridge.prototype.execFunction server.js:183 16
11:32:46     INFO -         Session.prototype.receive server.js:283 3
11:32:46     INFO -         AsyncRead.prototype.onDataAvailable server.js:88 3
11:32:46     INFO -  SUMMARY-UNEXPECTED-FAIL | test-attachment.js | test-attachment.js::test_forward_raw_attachment
11:32:46     INFO -    EXCEPTION: cwc.window.document.documentElement.getButton is not a function
11:32:46     INFO -      at: test-attachment.js line 245
11:32:46     INFO -         subtest_open_attachment test-attachment.js:245 3
11:32:46     INFO -         Runner.prototype.wrapper frame.js:580 7
11:32:46     INFO -         startTest test-window-helpers.js:317 11
11:32:46     INFO -         sleep utils.js:402 5
11:32:46     INFO -         wait_for_compose_window test-compose-helpers.js:256 3
11:32:46     INFO -         open_compose_with_forward test-compose-helpers.js:159 10
11:32:46     INFO -         test_forward_raw_attachment test-attachment.js:275 13
11:32:46     INFO -         Runner.prototype.wrapper frame.js:585 9
11:32:46     INFO -         Runner.prototype._runTestModule frame.js:655 9
11:32:46     INFO -         Runner.prototype.runTestModule frame.js:701 3
11:32:46     INFO -         Runner.prototype.runTestDirectory frame.js:525 7
11:32:46     INFO -         runTestDirectory frame.js:707 3
11:32:46     INFO -         Bridge.prototype._execFunction server.js:179 10
11:32:46     INFO -         Bridge.prototype.execFunction server.js:183 16
11:32:46     INFO -         Session.prototype.receive server.js:283 3
11:32:46     INFO -         AsyncRead.prototype.onDataAvailable server.js:88 3
11:32:46     INFO -    EXCEPTION: Timeout waiting for window to close!
11:32:46     INFO -      at: utils.js line 447
11:32:46     INFO -         TimeoutError utils.js:447 13
11:32:46     INFO -         waitFor utils.js:485 11
11:32:46     INFO -         WindowWatcher_waitForWindowClose test-window-helpers.js:398 5
11:32:46     INFO -         wait_for_window_close test-window-helpers.js:639 3
11:32:46     INFO -         close_window test-window-helpers.js:650 3
11:32:46     INFO -         close_compose_window test-compose-helpers.js:191 5
11:32:46     INFO -         test_forward_raw_attachment test-attachment.js:279 3
11:32:46     INFO -         Runner.prototype.wrapper frame.js:585 9
11:32:46     INFO -         Runner.prototype._runTestModule frame.js:655 9
11:32:46     INFO -         Runner.prototype.runTestModule frame.js:701 3
11:32:46     INFO -         Runner.prototype.runTestDirectory frame.js:525 7
11:32:46     INFO -         runTestDirectory frame.js:707 3
11:32:46     INFO -         Bridge.prototype._execFunction server.js:179 10
11:32:46     INFO -         Bridge.prototype.execFunction server.js:183 16
11:32:46     INFO -         Session.prototype.receive server.js:283 3
11:32:46     INFO -         AsyncRead.prototype.onDataAvailable server.js:88 3

Or here:
https://archive.mozilla.org/pub/thunderbird/try-builds/mozilla@jorgk.com-2b410485bd8e/try-comm-central-linux64/try-comm-central_ubuntu64_vm_test-mozmill-bm121-tests1-linux64-build31.txt.gz

11:32:27     INFO -  SUMMARY-UNEXPECTED-FAIL | test-attachment.js | test-attachment.js::test_open_attachment
11:32:27     INFO -    EXCEPTION: Timeout waiting for modal dialog to open.
11:32:27     INFO -      at: utils.js line 447
11:32:27     INFO -         TimeoutError utils.js:447 13
11:32:27     INFO -         waitFor utils.js:485 11
11:32:27     INFO -         WindowWatcher_waitForModalDialog test-window-helpers.js:376 5
11:32:27     INFO -         wait_for_modal_dialog test-window-helpers.js:613 3
11:32:27     INFO -         test_open_attachment test-attachment.js:266 3
11:32:27     INFO -         Runner.prototype.wrapper frame.js:585 9
11:32:27     INFO -         Runner.prototype._runTestModule frame.js:655 9
11:32:27     INFO -         Runner.prototype.runTestModule frame.js:701 3
11:32:27     INFO -         Runner.prototype.runTestDirectory frame.js:525 7
11:32:27     INFO -         runTestDirectory frame.js:707 3
11:32:27     INFO -         Bridge.prototype._execFunction server.js:179 10
11:32:27     INFO -         Bridge.prototype.execFunction server.js:183 16
11:32:27     INFO -         Session.prototype.receive server.js:283 3
11:32:27     INFO -         AsyncRead.prototype.onDataAvailable server.js:88 3

Aceman, can you take a look please.
Flags: needinfo?(acelists)
(Assignee)

Comment 3

7 months ago
Regression time frame:
First bad: Sat Oct 29, 18:54:49 (Times in GMT+2)
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2b410485bd8e

Last good: Sat Oct 29, 10:56:12 (Times in GMT+2)
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f912cca79bb4

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1561c917ee27&tochange=969c3295d3aa
Sadly, I can't see anything obvious.
(Assignee)

Comment 4

7 months ago
mozmake SOLO_TEST=composition/test-attachment.js mozmill-one fails locally as expected. Same error as in comment #2 (I won't repeat it).

You don't actually need a test to look at this. Just start a new composition, drag a text file onto the addressing widget so the attachment bucket opens. Then double click this attachment to view it.

Nothing happens. On the console we see:

JavaScript error: , line 0: uncaught exception: 2153578497
[7304] WARNING: NS_ENSURE_TRUE(window) failed: file c:/mozilla-source/comm-central/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2570

The error is:
http://james-ross.co.uk/mozilla/misc/nserror?0x805D0001

Value	0x805D0001 (2153578497)
Name	NS_ERROR_WONT_HANDLE_CONTENT (search MXR, Google, Bing, Yahoo)
Module	URILOADER (24)

So it looks like a fallout from bug 1203813:
https://hg.mozilla.org/mozilla-central/rev/a1ea2703d9e8

The code that throws is
nsresult nsExternalAppHandler::MaybeCloseWindow()
{
  nsCOMPtr<nsPIDOMWindowOuter> window = do_GetInterface(mContentContext);
  NS_ENSURE_STATE(window); <<=== line 2570.

Mike, Blake, bug 1203813 is breaking TB. Attachments in the attachments bucked in the compose window can no longer be opened. Any hints?
Flags: needinfo?(mrbkap)
Flags: needinfo?(mconley)
(Assignee)

Updated

7 months ago
Summary: TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js → TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js - Attachment in attachments bucket in compose window don't open
(Assignee)

Updated

7 months ago
Summary: TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js - Attachment in attachments bucket in compose window don't open → TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js - Attachments in attachments bucket in compose window don't open
(Assignee)

Comment 5

7 months ago
Stack (not real useful):
nsExternalAppHandler::MaybeCloseWindow() Line 2570	C++
nsExternalAppHandler::OnStartRequest(nsIRequest * request, nsISupports * aCtxt) Line 1678	C++
nsDocumentOpenInfo::OnStartRequest(nsIRequest * request, nsISupports * aCtxt) Line 269	C++
nsBaseChannel::OnStartRequest(nsIRequest * request, nsISupports * ctxt) Line 809	C++
nsInputStreamPump::OnStateStart() Line 525	C++

JS stack (DumpJSStack()):
there is no JSContext on the stack!

That's a little tough :-(

I've just noticed that the DOM Inspector also doesn't work any more in this version. It launches once with an empty window, and then it doesn't launch any more. It still works in the last good version:
Last good: Sat Oct 29, 10:56:12 (Times in GMT+2)

Alice, can you do me a favour. Can you try in FF Nightlies whether the DOM Inspector still works.
For TB it stopped working in this range:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1561c917ee27&tochange=969c3295d3aa
Flags: needinfo?(alice0775)
(Assignee)

Updated

7 months ago
Blocks: 1313906

Comment 6

7 months ago
I can confirm that WARNING: NS_ENSURE_TRUE(window) failed: file c:/mozilla-source/comm-central/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2570

After this, if I attempt to attach another attachment in compose window, TB will crash with:
[9065] ###!!! ABORT: X_ShmAttach: BadAccess (attempt to access private resource denied); 4 requests ago; id=0x0
Re-running with MOZ_X_SYNC=1 in the environment may give a more helpful backtrace.: file /var/SSD/TB-hg/mozilla/toolkit/xre/nsX11ErrorHandler.cpp, line 147

#01: nsACString_internal::~nsACString_internal() (/var/SSD/TB-hg/tbird-bin/dist/include/nsTSubstring.h:95)
#02: g_logv (/usr/lib64/../lib64/libglib-2.0.so.0)
#03: g_log (/usr/lib64/../lib64/libglib-2.0.so.0)
#04: gdk_x11_register_standard_event_type (/usr/lib64/libgdk-3.so.0)
#05: gdk_x11_keymap_key_is_modifier (/usr/lib64/libgdk-3.so.0)
#06: _XError (/usr/lib64/../lib64/libX11.so.6)
#07: _XFreeX11XCBStructure (/usr/lib64/../lib64/libX11.so.6)
#08: _XFreeX11XCBStructure (/usr/lib64/../lib64/libX11.so.6)
#09: _XReply (/usr/lib64/../lib64/libX11.so.6)
#10: XGetWindowProperty (/usr/lib64/../lib64/libX11.so.6)
#11: gdk_x11_screen_supports_net_wm_hint (/usr/lib64/libgdk-3.so.0)
#12: gtk_widget_path_has_parent (/usr/lib64/libgtk-3.so.0)
#13: gtk_widget_path_has_parent (/usr/lib64/libgtk-3.so.0)
#14: gtk_window_set_focus (/usr/lib64/libgtk-3.so.0)
#15: g_closure_invoke (/usr/lib64/../lib64/libgobject-2.0.so.0)
#16: g_signal_handler_disconnect (/usr/lib64/../lib64/libgobject-2.0.so.0)
#17: g_signal_emit_valist (/usr/lib64/../lib64/libgobject-2.0.so.0)
#18: g_signal_emit (/usr/lib64/../lib64/libgobject-2.0.so.0)
#19: gtk_widget_show (/usr/lib64/libgtk-3.so.0)
#20: nsFilePicker::Open(nsIFilePickerShownCallback*) (/var/SSD/TB-hg/mozilla/widget/gtk/nsFilePicker.cpp:521)
#21: nsFilePicker::Show(short*) (/var/SSD/TB-hg/mozilla/widget/gtk/nsFilePicker.cpp:361)

But that may be just further fallout from the windows/dialogs not being what the app/gtk thinks they should be after the attachment click.
Flags: needinfo?(acelists)

Comment 7

7 months ago
Do the downloaded-file-open dialogs actually work in FF nightly?
(Assignee)

Comment 8

7 months ago
Moved the DOM Inspector issue to bug 1313906.

(In reply to :aceman from comment #7)
> Do the downloaded-file-open dialogs actually work in FF nightly?
I suppose they tested that. What's special about the attachment bucket?
But if there's a problem with the DOM Inspector also caused by bug 1203813, then there is a larger issue.
Blocks: 1203813
(Assignee)

Comment 9

7 months ago
Backing out https://hg.mozilla.org/mozilla-central/rev/a1ea2703d9e8 from bug 1203813 locally let's me open attachments in the compose window's attachment bucket again but the DOM inspector still doesn't launch. So that's a different issue.
Flags: needinfo?(alice0775)
(Assignee)

Updated

7 months ago
No longer blocks: 1313906

Comment 10

7 months ago
For me the attachment error sequence in the log is like this:
[22106] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file mozilla/netwerk/base/nsIOService.cpp, line 792
[22106] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file mozilla/netwerk/base/nsNetUtilInlines.h, line 180
[22106] WARNING: NS_ENSURE_TRUE(window) failed: file mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2570

So at first in nsIOService fails to call NewChannel2 and then also NewChannel() fails.

If I view a message with attachment (may be a draft) and there is the attachment list on the bottom of the preview, clicking the attachments there DOES work.

Jorg, can you continue from there? Maybe the compose window does not have the needed provileges/sandbox flags (as discussed in bug 1203813), but the main window has them. So needs to find out which are needed and apply them to compose window.
(Assignee)

Comment 11

7 months ago
Thanks for looking into it. After backing out https://hg.mozilla.org/mozilla-central/rev/a1ea2703d9e8 from bug 1203813 the attachment opens, however I still get

JavaScript error: , line 0: uncaught exception: 2153578497
[2464] WARNING: NS_ENSURE_TRUE(window) failed: file c:/mozilla-source/comm-central/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2570

in the console. So something is wrong, we only noticed it just now.

In my daily build of 2016-10-20 the attachments open, but error 2153578497 is reported in the error console. So as I said, bug 1203813 is most likely not causing the problem but only exposing or at best/worst aggravating it.

BTW, I don't see the debug you mentioned in comment #10.

Any good idea how to find
  JavaScript error: , line 0: uncaught exception: 2153578497

Comment 12

7 months ago
I see those message in the console (linux terminal) in a debug build.

I would put a debug output in nsNetUtilInlines.h in the function NS_NewChannelInternal() to see what do the *Principal and SecurityFlags variables contain. I could show the difference between opening the attachment dialog from the msg preview (works) and compose window (does not work).
(Assignee)

Comment 13

7 months ago
Opening attachments in the compose window and the message attachment pane are different. The former is handled in OpenSelectedAttachment() in MsgComposeCommands.js.

The change in bug 1203813 seems to shift the window from the attachment bucked to the overall compose window.
(Assignee)

Comment 14

7 months ago
This indeed seems to be some security issue.
File attachments in the attachment bucket need access to disk files via file-URIs. Attached message in the attachment bucket or attachments in received messages or drafts access mailnews URLs, that is message MIME parts, not disk files.
Summary: TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js - Attachments in attachments bucket in compose window don't open → TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js - File attachments in attachments bucket in compose window don't open
(Assignee)

Comment 15

7 months ago
(In reply to Jorg K (GMT+2) from comment #14)
> This indeed seems to be some security issue.
> File attachments in the attachment bucket need access to disk files via
> file-URIs. Attached message in the attachment bucket or attachments in
> received messages or drafts access mailnews URLs, that is message MIME
> parts, not disk files.

I give up for now. Since (the somewhat unrelated) bug 1203813 landed, we can't open file-based attachments any more from the compose windows attachments bucket. Processing is here:

https://dxr.mozilla.org/comm-central/rev/6b8c7cd17855f7de5eccb0ea9efcd5dbccc68573/mail/components/compose/content/MsgComposeCommands.js#4256

Debug shows the URI to be opened as for example |file:///D:/Desktop/script.txt| and nothing opens.

Error 0x805D0001 (2153578497) is also displayed without the patch from bug 1203813, but at least the attachment opens.

Older successful Mozmill run for example show:
JavaScript error: resource://mozmill/modules/utils.js, line 402: uncaught exception: 2153578497
[3088] WARNING: NS_ENSURE_TRUE(window) failed: file c:/builds/moz2_slave/tb-try-c-cen-w32-d-00000000000/build/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2570

So something is wrong with the call to newChannelFromURI2(). What should the security flags be so we can open a file? Who knows? Maybe Honza?
Flags: needinfo?(honzab.moz)
(Assignee)

Updated

7 months ago
status-thunderbird52: --- → affected
tracking-thunderbird52: --- → +

Comment 16

7 months ago
(In reply to Jorg K (GMT+2) from comment #15)
> So something is wrong with the call to newChannelFromURI2(). What should the
> security flags be so we can open a file? Who knows? Maybe Honza?

Can they be determined with the process from comment 12?
(Assignee)

Comment 17

7 months ago
Once again, Mozmill runs from 15th Oct 2016 show:
JavaScript error: resource://mozmill/modules/utils.js, line 402: uncaught exception: 2153578497

This is generated here trying to open a file from the attachment bucket:
https://dxr.mozilla.org/comm-central/rev/6b8c7cd17855f7de5eccb0ea9efcd5dbccc68573/mail/components/mailContentHandler.js#56

So that seems to be the "normal" way for the content handler to communicate that it doesn't want to handle the content. Adding "file" to the if-clause just above makes the throw go away and for some file I get a panel to choose an application to open the file with.

The content handler is called by nsDocumentOpenInfo::DispatchContent() here:
https://dxr.mozilla.org/mozilla-central/rev/969c3295d3aa77931cca26eddb047d9d74bd9858/uriloader/base/nsURILoader.cpp#462
and NS_ERROR_WONT_HANDLE_CONTENT is quite expected there.

Summarising:
Since returning NS_ERROR_WONT_HANDLE_CONTENT from the content handler is quite expected and the JavaScript error is just a confusing message that was always shown (I assume bad XPCOM handling), I need to come back to the two main questions:

1) Why has https://hg.mozilla.org/mozilla-central/rev/a1ea2703d9e8 from
   bug 1203813 caused this problem?
2) Should anything be changed in the call to newChannelFromURI2() here:
   https://dxr.mozilla.org/comm-central/rev/6b8c7cd17855f7de5eccb0ea9efcd5dbccc68573/mail/components/compose/content/MsgComposeCommands.js#4256
(Assignee)

Comment 18

7 months ago
(In reply to :aceman from comment #16)
> Can they be determined with the process from comment 12?

We've been barking up the wrong tree.

Once again: Message preview and attachment bucket access completely different schemes: The former opens mailnews URLs (like mailbox://, imap://, etc.), the latter opens file-URLs. There are completely different security requirements.

JavaScript error: resource://mozmill/modules/utils.js, line 402: uncaught exception: 2153578497
is normal and expected and so is:
[2464] WARNING: NS_ENSURE_TRUE(window) failed: file c:/mozilla-source/comm-central/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp, line 2570

I've tried in TB 45. Opening a file attachment from the attachment bucket in the compose window gives
  Error: uncaught exception: 2153578497
in the error console. Sad, but true. And we understand now where it comes from.

That "background noise" has always been there. The only thing that has changed by bug 1203813 is that the window that's used to open something
  https://hg.mozilla.org/mozilla-central/rev/a1ea2703d9e8#l1.24
has changed.

Why that makes a difference to the ability to open a file-URL I don't understand.

I think instead of wasting my time poking around the system that has always reported strange errors, I'd rather wait for an answer from Mike.

Comment 19

7 months ago
The info in bug 1044109 may explain the uncaught exception message.

There's some documentation about content policy types etc here http://searchfox.org/mozilla-central/source/dom/base/nsIContentPolicyBase.idl#65 and here https://dxr.mozilla.org/comm-central/source/mozilla/netwerk/base/nsILoadInfo.idl#47

The comments in the bug may also be useful e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=1203813#c8
(Assignee)

Comment 20

7 months ago
Magnus, who are our experts on this stuff?

Thanks Aleth, I've been through the IDL files you indicated this afternoon. SEC_NORMAL, that we use, is actually deprecated. I tried some other security flags, and I could make the application crash on opening the attachment, but I couldn't get it to open. (Now I have other things to do instead of poking around stuff I don't understand, and none of the people who have commented on the bug don't understand either).
Flags: needinfo?(mkmelin+mozilla)

Comment 21

7 months ago
Did you check if rootwin here https://hg.mozilla.org/mozilla-central/rev/a1ea2703d9e8#l1.24 gets a reasonable value in this case?
(Assignee)

Comment 22

7 months ago
By the looks of it, the TB compose window attachment bucket is an unexpected use of toolkit/mozapps/downloads/nsHelperAppDlg.js. I personally don't quite understand how we end up there.

Inspired by comment #21 I'll try:
      this.mDialog = ww.openWindow(rootWin ? rootWin :
                                   ir.getInterface(Components.interfaces.nsIDOMWindow),
where ir.getInterface(...) is what was passed in previously.

Comment 23

7 months ago
(In reply to Jorg K (GMT+2) from comment #20)
> Magnus, who are our experts on this stuff?

Isn't squib our attachments expert?
Flags: needinfo?(mkmelin+mozilla)
(Assignee)

Updated

7 months ago
status-thunderbird52: affected → ---
tracking-thunderbird52: + → ---
Component: General → DOM: Core & HTML
Flags: needinfo?(mrbkap)
Flags: needinfo?(mconley)
Flags: needinfo?(honzab.moz)
Keywords: intermittent-failure
OS: Linux → All
Product: Thunderbird → Core
Summary: TEST-UNEXPECTED-FAIL | /builds/slave/test/build/tests/mozmill/composition/test-attachment.js | test-attachment.js - File attachments in attachments bucket in compose window don't open → File attachments in attachments bucket in compose window don't open (breakage from bug 1203813)
(Assignee)

Updated

7 months ago
Assignee: nobody → jorgk
Status: NEW → ASSIGNED
Summary: File attachments in attachments bucket in compose window don't open (breakage from bug 1203813) → Thunderbird: File attachments in attachments bucket in compose window don't open (breakage from bug 1203813)
(Assignee)

Comment 24

7 months ago
Created attachment 8805872 [details] [diff] [review]
1279327.patch

This works for Thunderbird and should affect the desired behaviour implemented in bug 1203813. There might of course be a better solution.
Attachment #8805872 - Flags: review?(mconley)
Comment on attachment 8805872 [details] [diff] [review]
1279327.patch

Hm. I actually think the better solution is that we need to make sure that the context can resolve to an nsIDocShell.

See attached.
Attachment #8805872 - Flags: review?(mconley) → review-
Created attachment 8806081 [details] [diff] [review]
Potential fix for TB attachment bug
Does my attachment address the test failure?
Flags: needinfo?(jorgk)
(Assignee)

Comment 28

7 months ago
Yes, it does. Thanks! (I could have saved myself a lot of time digging around.)

Try run here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ae3df8870e9f4341806d80ec6fc14089210acf53

Could you please provide a commit message so I can r+ it and land it. My proposal would be:
Bug 1279327 - Port bug 1203813 to TB: Allow nsAttachmentOpener to resolve to nsIDocShell.

Should the else case be maintained?
-    else
-      return this.QueryInterface(iid);
Flags: needinfo?(jorgk)
(Assignee)

Updated

7 months ago
Attachment #8805872 - Attachment is obsolete: true
(Assignee)

Updated

7 months ago
Component: DOM: Core & HTML → Message Compose Window
Product: Core → Thunderbird
Created attachment 8806107 [details] [diff] [review]
Port bug 1203813 to TB: Allow nsAttachmentOpener to resolve to nsIDocShell

Updated

7 months ago
Attachment #8806081 - Attachment is obsolete: true

Updated

7 months ago
Attachment #8806107 - Flags: review?(jorgk)
(Assignee)

Comment 30

7 months ago
Comment on attachment 8806107 [details] [diff] [review]
Port bug 1203813 to TB: Allow nsAttachmentOpener to resolve to nsIDocShell

Thanks again. I was going to suggest to add more braces {}, but it's happened already ;-)

I'll take care of the try run and landing it. No need to put checkin-needed, it's under control.
Attachment #8806107 - Flags: review?(jorgk) → review+
Cool, thanks!

Comment 32

7 months ago
So if this is fixed on the TB part, we'll need a SM version too.
(Assignee)

Comment 33

7 months ago
https://hg.mozilla.org/comm-central/rev/da55a6abf6f90467f65ded069cd0dee5e45119c2
Landed on closed/busted tree, but with green try run here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1c3ba5cec8596d897b820ff39436a496a05acf6c

FRG or Rsx11m, this needs porting to SM.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
Flags: needinfo?(rsx11m.pub)
Flags: needinfo?(frgrahl)
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 52.0

Comment 34

7 months ago
As far as I can see SM's OpenSelectedAttachment() does things very differently from TB's version, so does not need anything porting.
(Assignee)

Comment 35

7 months ago
Right:
TB:
uriLoader.openURI(channel, true, new nsAttachmentOpener());
Patch was in nsAttachmentOpener.
SM:
editorElement.webNavigation.loadURI(attachmentUrl, loadFlags, null, null, null);
No patch.

Maybe you want to check it still works anyway.

Comment 36

7 months ago
Looks like its still working with yesterdays build. Thanks for the heads up. Will check again with a fresh build but need to fix the installer again first. Bug 1307456 removed two xpts.
Flags: needinfo?(rsx11m.pub)
Flags: needinfo?(frgrahl)
You need to log in before you can comment on or make changes to this bug.