Last Comment Bug 482659 - [FIX]AOL.com/AIM.com inline/attached images don't display
: [FIX]AOL.com/AIM.com inline/attached images don't display
Status: VERIFIED FIXED
[READ comment 55 before commenting]
: common-issue-, regression, verified1.9.0.9, verified1.9.1
Product: Core
Classification: Components
Component: Layout: Images (show other bugs)
: Trunk
: x86 All
: P1 major (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
Mentors:
http://www.aol.com
: 481674 482746 483860 (view as bug list)
Depends on: 483748
Blocks: 445004 465804
  Show dependency treegraph
 
Reported: 2009-03-10 21:50 PDT by Stephen Donner [:stephend]
Modified: 2009-08-27 11:00 PDT (History)
23 users (show)
mbeltzner: blocking1.9.1+
dveditz: blocking1.9.0.9+
samuel.sidler+old: wanted1.9.0.x+
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot of 3.0.6 (465.75 KB, image/png)
2009-03-10 21:50 PDT, Stephen Donner [:stephend]
no flags Details
Screenshot of 3.0.7 (323.38 KB, image/png)
2009-03-10 21:50 PDT, Stephen Donner [:stephend]
no flags Details
testcase (671 bytes, text/html)
2009-03-12 21:29 PDT, Nochum Sossonko [:Natch]
no flags Details
This gives about:blank URIs the loader's base URI (19.95 KB, patch)
2009-03-13 13:01 PDT, Boris Zbarsky [:bz] (still a bit busy)
dcamp: review+
jst: superreview+
Details | Diff | Splinter Review
1.9.0 patch (19.78 KB, patch)
2009-03-17 06:44 PDT, Boris Zbarsky [:bz] (still a bit busy)
no flags Details | Diff | Splinter Review
1.9.0 patch with the test commented out (18.18 KB, patch)
2009-03-17 10:10 PDT, Boris Zbarsky [:bz] (still a bit busy)
dveditz: approval1.9.0.9+
Details | Diff | Splinter Review

Description Stephen Donner [:stephend] 2009-03-10 21:50:27 PDT
Created attachment 366755 [details]
Screenshot of 3.0.6

Summary: AOL.com/AIM.com attached images aren't displaying inline.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 works

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.7) Gecko/2009021910 Firefox/3.0.7 fails

Steps to Reproduce:

1. Using the 3.0.7 build, log in to AOL.com or AIM.com
2. Load any email that has an attached, but normally inline-viewable image.

Expected Results:

Image displays

Actual Results:

Broken image icon displays; image is still there in the email content, though.

It's content-type is:

Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="2008_Lexus_IS_F_113_(400x300).jpg"
Content-Type: image/jpeg; name="2008_Lexus_IS_F_113_(400x300).jpg"

Sorry, I don't yet have time for a regression range.
Comment 1 Stephen Donner [:stephend] 2009-03-10 21:50:58 PDT
Created attachment 366756 [details]
Screenshot of 3.0.7
Comment 2 Samuel Sidler (old account; do not CC) 2009-03-10 22:04:07 PDT
Stephen says this is broken in Shiretoko too. I'll test this more tomorrow, but nominating for a bunch of things so it gets on the radar.
Comment 3 Stephen Donner [:stephend] 2009-03-10 22:10:51 PDT
(In reply to comment #2)
> Stephen says this is broken in Shiretoko too. I'll test this more tomorrow, but
> nominating for a bunch of things so it gets on the radar.

Yeah, broken in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b4pre) Gecko/20090310 Shiretoko/3.1b4pre, too; I won't belabour you with more screenshots.
Comment 4 Daniel Veditz [:dveditz] 2009-03-11 00:06:30 PDT
Any messages in the error console? (make sure javascript.options.showInConsole is enabled)

Does the problem reproduce if you "Save as Complete" and then load it locally? If so we could pass those files around and wouldn't have to all get AOL mail accounts to test the problem, and whittle it down to a minimal testcase we could stick in the regression testsuite.
Comment 5 Nochum Sossonko [:Natch] 2009-03-11 04:34:33 PDT
It seems that relative urls starting with "/" don't map correctly, that's what is breaking the images on aol, with a full path it works fine. I tested this locally as well, and I'll try to get a testcase up soon.
Comment 6 Matthias Versen [:Matti] 2009-03-11 10:29:16 PDT
*** Bug 481674 has been marked as a duplicate of this bug. ***
Comment 7 Matthias Versen [:Matti] 2009-03-11 10:30:35 PDT
see bug 482659 for some information (relative URL, page source, image properties)
Comment 8 Samuel Sidler (old account; do not CC) 2009-03-11 10:44:03 PDT
*** Bug 482746 has been marked as a duplicate of this bug. ***
Comment 9 Samuel Sidler (old account; do not CC) 2009-03-11 10:44:40 PDT
We'll probably need to block on this since it's now a common issue.
Comment 10 Mike Beltzner [:beltzner, not reading bugmail] 2009-03-11 11:20:42 PDT
(In reply to comment #7)
> see bug 482659 for some information (relative URL, page source, image

This is bug 482659.

--> Core::ImgLib
Comment 11 Matthias Versen [:Matti] 2009-03-11 11:30:00 PDT
I meant bug 481674...
Comment 12 Joe Drew (not getting mail) 2009-03-11 12:34:11 PDT
imgLoader::LoadImage() is never called for those attached images, so the problem does not lie in imagelib.

(I have a test AOL account whose credentials I will pass out to people who ask. E-mail me privately.)
Comment 13 Samuel Sidler (old account; do not CC) 2009-03-11 14:24:36 PDT
(This problem also appears for AIM Mail accounts, which are freely available and are automatically created for anyone with an AIM account.)
Comment 14 Joe Drew (not getting mail) 2009-03-11 15:13:02 PDT
Right. I was just trying to save someone from the (considerable!) annoyance of finding a screen name that is a) memorable and b) not taken. Fwiw, all I had to do to reproduce the bug was send a plain text message with an image as an attachment.
Comment 15 Samuel Sidler (old account; do not CC) 2009-03-11 18:52:11 PDT
Do we know what regressed this?
Comment 16 Matthew Middleton (:zzxc) 2009-03-12 19:22:22 PDT
Looking at the DOM of the page, it appears that the cause of this is that URL, baseURI, and documentURI are "about:blank" in the message <iframe>'s document.  This is breaking the relative image paths used for attachments.

From a quick scan of the 1.9.0.7 changes, bug 459470 could be related.
Comment 17 Samuel Sidler (old account; do not CC) 2009-03-12 21:03:14 PDT
Boris, Jonas: Any idea here?
Comment 18 Nochum Sossonko [:Natch] 2009-03-12 21:29:30 PDT
Created attachment 367172 [details]
testcase

Thanks Matthew, here's a reduced testcase.
Comment 19 Nochum Sossonko [:Natch] 2009-03-12 22:34:03 PDT
Comment on attachment 367172 [details]
testcase

This is wrong, although that is what shows up on AOL's frame. Bug 436965 is also a possible candidate... I'll try to narrow down some regression range.
Comment 20 Nochum Sossonko [:Natch] 2009-03-12 23:17:34 PDT
Regression Range:

08-11-18 works
08-11-19 fails

pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2d4ddc92e47d&tochange=0cd41f599080

->Bug 445004
Comment 21 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-03-13 06:35:59 PDT
I think I nailed the testcase down to this: http://martijn.martijn.googlepages.com/aol_test.htm
This works in Firefox 3.0.6, but doesn't work in Firefox3.0.7.
Comment 22 Boris Zbarsky [:bz] (still a bit busy) 2009-03-13 06:39:02 PDT
Yeah, so the key change here for bug 445004 was to change the URI and base URI used for document.written documents. They used to have something to do with the document the function calling document.write() was defined in.  That bug changed it to the document the script is running on (which is not the same thing, in some cases).

I did do some testing of IE's behavior wrt document.write() (see bug 445004 comment 24) and implemented something compatible with that testing.  But perhaps IE has some sort of special-case behavior for situations where the document the script is running on is about:blank...  Martijn, thanks for the testcase.  I'll try to dig into it a bit and see what I can get from it.
Comment 23 Boris Zbarsky [:bz] (still a bit busy) 2009-03-13 09:46:47 PDT
OK, I see what's going on.  The javascript: URI itself does have the right base URI here, but at the point when the write() happens the base URI of the subframe really is just about:blank.

On Martijn's testcase, I see the following results:

Safari 3.2.1: shows image
Webkit trunk: does not show image
Opera: does not show image
IE: shows image

Doing more digging now.
Comment 24 Boris Zbarsky [:bz] (still a bit busy) 2009-03-13 09:50:55 PDT
Er, I take that back.  On Martijn's testcase, IE7 does NOT show the image.
Comment 25 Boris Zbarsky [:bz] (still a bit busy) 2009-03-13 10:01:11 PDT
And I take that back too; if I change the testcase enough (work around IE's bogus handling of <html:button> and its inability to remove the load handler), then the image does show up in IE.

And the key here is that in IE the base URI of the about:blank document is NOT about:blank.  In particular, the image shows up even if I remove the location set from the testcase.  I guess I knew that; see bug 465804 comment 2.  That didn't use to work in Gecko, of course, and I wonder whether the site is using the location set to work around that...  Martijn, do they really just write out the empty string in the original site?
Comment 26 Boris Zbarsky [:bz] (still a bit busy) 2009-03-13 13:01:37 PDT
Created attachment 367278 [details] [diff] [review]
This gives about:blank URIs the loader's base URI

Not completely sure how safe this is, but I think it should be ok; it's aligning us closer to IE on this matter.  This also affects the base URI of window.open("about:blank"), but NOT window.open("").  In IE the latter has the base URI of the opening document.  If desired, I can fix that case too.

biesi says he's ok with the general necko approach and with dcamp doing the review.

jst, what do you think of this in web compat terms?
Comment 27 Boris Zbarsky [:bz] (still a bit busy) 2009-03-13 13:03:01 PDT
Setting P1; beta exposure would be afwul nice here.
Comment 28 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-03-16 05:23:54 PDT
I've verified in my debug build that the patch fixes the AOL mail issue.
Comment 29 Daniel Veditz [:dveditz] 2009-03-16 14:37:03 PDT
I'm a little worried about what new thing this patch will break, but I don't see how we can not un-break the AOL/AIM users we screwed over with 3.0.7 --> blocking1.9.0.8
Comment 30 Boris Zbarsky [:bz] (still a bit busy) 2009-03-16 18:01:40 PDT
Pushed http://hg.mozilla.org/mozilla-central/rev/55d159b75f41
Comment 31 Boris Zbarsky [:bz] (still a bit busy) 2009-03-16 18:06:09 PDT
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/21476a11e9f0
Comment 32 Boris Zbarsky [:bz] (still a bit busy) 2009-03-16 18:06:22 PDT
Comment on attachment 367278 [details] [diff] [review]
This gives about:blank URIs the loader's base URI

Requesting branch approval
Comment 34 Dave Townsend [:mossop] 2009-03-17 04:09:56 PDT
Right and I didn't notice that this hadn't baked on trunk and so was breaking there too. Totally backed out.
Comment 35 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 06:39:01 PDT
Hmm.  I'd actually run tests on that patch, I thought...

In any case, the problem is stomping on |rv| in the non-nested case in nsAboutProtocolHandler::NewChannel.  Fixed that, and relanded on m-c: http://hg.mozilla.org/mozilla-central/rev/69aa67fe3314
Comment 36 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 06:40:44 PDT
And on 1.9.1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/780e244b9b8e

The leaks are an interesting question.  I'll look into them; I bet some code somewhere just totally fails to deal with newChannel failing... which we should fix regardless of this bug.
Comment 37 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 06:44:12 PDT
Created attachment 367766 [details] [diff] [review]
1.9.0 patch
Comment 38 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 08:33:48 PDT
Ah, the leaks is due to NS_NewChannel assuming that the protocol handler's newChannel didn't touch the out param on failure.  Filed bug 483788 on that.
Comment 39 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 08:55:17 PDT
Er, and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/01d135d82e30 to fix the fact that I forgot to qref on 1.9.1.
Comment 40 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 10:09:02 PDT
OK, the remaining leak failure was due to bug 483818 being tickled by the test.  Pushed http://hg.mozilla.org/mozilla-central/rev/66e869462475 and http://hg.mozilla.org/releases/mozilla-1.9.1/rev/73236cfcbde9 to disable that part of the test.
Comment 41 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 10:10:41 PDT
Created attachment 367799 [details] [diff] [review]
1.9.0 patch with the test commented out
Comment 42 Daniel Veditz [:dveditz] 2009-03-17 13:42:40 PDT
Comment on attachment 367799 [details] [diff] [review]
1.9.0 patch with the test commented out

Approved for 1.9.0.8, a=dveditz for release-drivers
Comment 43 Nochum Sossonko [:Natch] 2009-03-17 13:44:45 PDT
*** Bug 483860 has been marked as a duplicate of this bug. ***
Comment 44 Boris Zbarsky [:bz] (still a bit busy) 2009-03-17 16:27:33 PDT
Checked into CVS.
Comment 45 Stephen Donner [:stephend] 2009-03-17 16:57:04 PDT
Verified FIXED in 1.9.1 using:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b4pre) Gecko/20090317 Shiretoko/3.5b4pre

Replacing fixed1.9.1 keyword with verified1.9.1
Comment 46 Stephen Donner [:stephend] 2009-03-17 21:03:30 PDT
Also verified FIXED on trunk using:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090317 Minefield/3.6a1pre
Comment 47 noodnick9 2009-03-22 19:29:25 PDT
Any idea when this fix will be available to us users?  
I have dropped back to 3.0.6 so that attachments will display, and can't move to 3.0.7 because of this bug.  Thanks.
Comment 48 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-03-23 03:18:57 PDT
It should be available in the next security update, which is 3.0.8.
Comment 49 noodnick9 2009-03-23 04:28:58 PDT
OK, thanks.   Any idea when that will be?
Comment 50 Martijn Wargers [:mwargers] (not working for Mozilla) 2009-03-23 04:45:59 PDT
My estimate would be something like approximately 6 weeks.
Comment 51 Samuel Sidler (old account; do not CC) 2009-03-23 07:07:38 PDT
We're targeting a release of Firefox 3.0.8 at or around April 14.

https://wiki.mozilla.org/Releases
Comment 52 noodnick9 2009-03-23 10:42:08 PDT
Wonderful.  Thank you.
Comment 53 Al Billings [:abillings] 2009-03-24 12:55:08 PDT
Verified in 1.9.0.8 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.8pre) Gecko/2009031904 GranParadiso/3.0.8pre.
Comment 54 tommyrockt 2009-03-24 18:07:45 PDT
As of 3/24/09 the AOL image issue is still unresolved in Firefox 3.0.7
Comment 55 Samuel Sidler (old account; do not CC) 2009-03-24 18:23:12 PDT
This issue will be fixed in Firefox 3.0.8, targeted for release in mid-April.
Comment 56 [:Cww] 2009-08-27 11:00:49 PDT
Don't see this any more.

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