No authorisation prompt displayed when inserting image into email body if image URL requires authentication (regression TB 52)

RESOLVED FIXED in Thunderbird 57.0

Status

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: adepty, Unassigned)

Tracking

({regression})

52 Branch
Thunderbird 57.0
regression
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird_esr5255+ fixed, thunderbird55 wontfix, thunderbird56 fixed, thunderbird57 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

a year ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.9) Gecko/20100101 Goanna/3.2 Firefox/45.9 PaleMoon/27.3.0
Build ID: 20170422232241

Steps to reproduce:

1. Write new email
2. Insert -> Image
3. Paste URL to an image that require authentication (returns HTTP/1.1 401 Unauthorized)





Actual results:

It'll not be able to fetch it (broken image icon). It worked before update to 52.1.1. I think it can be connected with data URIs for images


Expected results:

I expect that image will be downloaded and attached to email (like it worked in thunderbird 45.8.0)
(Reporter)

Comment 1

a year ago
I've tested in Thunderbird 54 beta and face the same issue

Comment 2

a year ago
Images from http(s) resources are not converted to data: URLs, so this should not be connected to the changes we made.

However, I set this up. I placed a picture here:
http://www.jorgk.com/auth/ausflag.png and this needs authorisation (which is guest1/guest1). In TB 45 I get prompted when inserting the image, in TB 52 there is no prompt.

I'm not aware of breaking this with the changes we made to the compose window, but maybe we did.

Alice, can you find the regression here:
As I said, write an e-mail, insert an image from http://www.jorgk.com/auth/ausflag.png and enter guest1/guest1 if prompted.

The bug that might have broken it handed here, but I doubt it:
https://hg.mozilla.org/comm-central/rev/14bd3b70a7fda2bfb80b0905adb05ceef296e2d4

Or perhaps not being an editor any more will stop the prompt.
Flags: needinfo?(alice0775)
Summary: After update to Thunderbird 52.1.1 it's not possible to insert image in email body if image URL require authentication → After update to Thunderbird 52.1.1 it's not possible to insert image in email body if image URL require authentication - no authorisation prompt displayed

Comment 3

a year ago
Workaround: Enter, for example, http://guest1:guest1@www.jorgk.com/auth/ausflag.png as the image name ;-)

Comment 4

a year ago
good:http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1472724143/thunderbird-51.0a1.en-US.win32.zip
bad :http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1472768360/thunderbird-51.0a1.en-US.win32.zip

https://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2016-09-01+03%3A00%3A00&enddate=2016-09-01+16%3A00%3A00
https://hg.mozilla.org/comm-central/pushloghtml?fromchange=cddb63dcd5cf51f63d01e4c01c551c458765f042&tochange=f3f22e07b8e8beb02592cd68c83267fe570e59e9

Suspect: Bug 1266836



Error in Error Console:

GET 
http://www.jorgk.com/auth/ausflag.png [HTTP/1.1 401 Unauthorized 285ms]

TypeError: win.gBrowser is undefined[Learn More]  nsLoginManagerPrompter.js:1426:11

nsPrompter: Delegation to password manager failed: [Exception... "[JavaScript Error: "win.gBrowser is undefined" {file: "resource://gre/components/nsLoginManagerPrompter.js" line: 1426}]'[JavaScript Error: "win.gBrowser is undefined" {file: "resource://gre/components/nsLoginManagerPrompter.js" line: 1426}]' when calling method: [nsIPromptFactory::getPrompt]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "JS frame :: resource://gre/components/nsPrompter.js :: getPrompt :: line 42"  data: yes]  nsPrompter.js:44

uncaught exception: 2147500033  nsPrompter.js:363:5

TypeError: checkValue is undefined[Learn More]  nsPrompter.js:740:13
Flags: needinfo?(alice0775)

Comment 6

a year ago
FRG, any idea what we need to do to get this going in the compose window? What happens when you insert
  http://www.jorgk.com/auth/ausflag.png
into a new e-mail composition in SM?
Flags: needinfo?(frgrahl)
(Reporter)

Comment 7

a year ago
Guys, thanks for so fast replies!
And for workaround :)
>> TypeError: win.gBrowser is undefined[Learn More]  nsLoginManagerPrompter.js:1426:11

I have run into this in SeaMonkey too. See bug 1347857. The autocomplete code was changed and tries to get a parent browser somewhere. This doesn't work because mail in both TB and SM hasn't a browser.

I just hadn't time yet to investigate further and for SeaMonkey there is a workaround but a change in mozilla code is probably needed. I just wonder why you ran into it in the current ESR only? Maybe my conclusion is/was wrong and Mike can shed some light on this. Overall its a little over my pay grade aka. not enough knowledge :)
Flags: needinfo?(frgrahl) → needinfo?(mconley)
I forgot. While this isn't autocomplete the password manager code was changed too here.

Comment 10

a year ago
According to comment #4 this actually regressed in TB 51 already and no one notices it.
status-thunderbird54: --- → affected
status-thunderbird55: --- → affected
status-thunderbird_esr52: --- → affected
tracking-thunderbird_esr52: --- → +
Summary: After update to Thunderbird 52.1.1 it's not possible to insert image in email body if image URL require authentication - no authorisation prompt displayed → No authorisation prompt displayed when inserting image into email body if image URL requires authentication (regression TB 52)
> this actually regressed in TB 51 already and no one notices it.

Then it fits. Autocomplete and the login prompter code was changed for Gecko 51.
See Also: → bug 1347857

Comment 12

a year ago
I'm sure Mike knows everything but perhaps the author of bug 1266836 can help us as well. Johann, what do we need to do to get the authorisation prompt back for the Thunderbird compose window?
Flags: needinfo?(jhofmann)
Yup, sorry, this is tracked in bug 1350152. Mike Kaply agreed to take the bug but according to him all consumers had worked around the issue already. I assume that's not the case for Thunderbird, then?

Would you mind chiming in on bug 1350152? FWIW the bug shouldn't be hard to solve (see comments) and I'm happy to mentor/review but I don't really have the resources to set up testing on Thunderbird or another XUL app right now.

Thank you, and again sorry for breaking basic auth for you :)
Status: UNCONFIRMED → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(mconley)
Flags: needinfo?(jhofmann)
Resolution: --- → DUPLICATE
Duplicate of bug: 1350152
Actually since this is in Thunderbird depending on bug 1350152 might be a better choice than duping...
Status: RESOLVED → REOPENED
Depends on: 1350152
Ever confirmed: true
Resolution: DUPLICATE → ---

Comment 15

a year ago
(In reply to Johann Hofmann [:johannh] from comment #13)
> FWIW the bug shouldn't be hard to solve (see comments) ...
Sure if you know where to start and what you're doing. You need to understand the situation of the TB team. We need to be experts in e-mail and also experts in *most* areas of Mozilla core since we interface with most of them. So we need to be experts in DOM, Editor, Intl, Netwerk, Cache2, just to name a few. In short: It's an impossible task.

> ... and I'm happy to mentor/review but I don't really have
> the resources to set up testing on Thunderbird or another XUL app right now.
Thanks but I wouldn't know where to start.

Bug 1350152 comment #5 talks about (quote): "bringing back code doing `….chromeEventHandler.ownerDocument.defaultView` as a fallback"

Mike, are you still offering to fix this?
Status: REOPENED → NEW
Flags: needinfo?(mozilla)

Updated

a year ago
Component: Untriaged → Security

Comment 16

a year ago
Created attachment 8872347 [details] [diff] [review]
1367191-wip.patch - Parch by FRG

Frank-Rainer Grahl, the SeaMonkey maintainer sent me this patch. If fixes the problem in Thunderbird.
Attachment #8872347 - Flags: feedback?(jhofmann)

Comment 17

a year ago
Let's try again with better grammar:
Frank-Rainer Grahl, the SeaMonkey maintainer, sent me this patch. It fixes the problem in Thunderbird.
Its just the old code based on the above comment:

>> Bug 1350152 comment #5 talks about (quote): "bringing back code doing 
>> `….chromeEventHandler.ownerDocument.defaultView` as a fallback"

It also fixed SeaMonkey bug 1347857 for me but I did only very limited testing.
Oh btw. can't be used 1:1 because it doesn't return a browser. This would need to be fixed.

FRG
Created attachment 8872356 [details] [diff] [review]
1367191-wip2.patch

Could you try this one.
Attachment #8872347 - Attachment is obsolete: true
Attachment #8872347 - Flags: feedback?(jhofmann)
Flags: needinfo?(jorgk)

Comment 21

a year ago
That works, too.
Flags: needinfo?(jorgk)
Great. Now someone more familiar with the core stuff needs to take a look.
Flags: needinfo?(jhofmann)
No, I don't have time to work on this. When I first took a look, it didn't seem as simple as I thought.
Flags: needinfo?(mozilla)
I'm looking into bug 1350152 and we might solve this soon.
Flags: needinfo?(jhofmann)

Comment 25

a year ago
Reporter: Can you test this on today's Daily (will be compiled in a few hours) now that bug 1350152 is fixed.

We'll see how to backport this to TB 52 ESR.

Comment 26

a year ago
Fixed by bug 1350152, I've just tried it.
Status: NEW → RESOLVED
Last Resolved: a year agoa year ago
status-thunderbird56: --- → affected
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 57.0

Updated

a year ago
Attachment #8872356 - Attachment is obsolete: true

Comment 27

a year ago
Created attachment 8893999 [details]
Don't forget to uplift M-C change to M-ESR52
Attachment #8893999 - Flags: approval-comm-esr52+
(Reporter)

Comment 28

a year ago
Hello
I can confirm that it's fixed in 57.0a1 (2017-08-04) (64-bit)
Thank you guys! Really appreciate it!
Created attachment 8894214 [details] [diff] [review]
1350152-ESR52.patch

Rebased ESR 52 version of bug 1350152 for ESR52 SeaMonkey and TB release branches.

Comment 30

a year ago
Comment on attachment 8894214 [details] [diff] [review]
1350152-ESR52.patch

[Triage Comment]
Attachment #8894214 - Flags: approval-comm-esr52+

Updated

a year ago
Blocks: 1266836
Keywords: regression

Comment 32

a year ago
THUNDERBIRD_52_VERBRANCH:
https://hg.mozilla.org/releases/mozilla-esr52/rev/f822bda79c28da0ad93b67d46abcb66f6d5c8c92

Will be fixed in TB 52.3.
status-thunderbird54: affected → ---
status-thunderbird55: affected → wontfix
status-thunderbird56: affected → fixed
status-thunderbird57: --- → fixed
status-thunderbird_esr52: affected → fixed
tracking-thunderbird_esr52: + → 55+

Updated

a year ago
Attachment #8893999 - Attachment is obsolete: true
Attachment #8893999 - Flags: approval-comm-esr52+

Comment 33

a year ago
Trick to get this off the list of things needing check-in: releases/comm-esr52/rev
You need to log in before you can comment on or make changes to this bug.