426029 - Media preview displays broken images for images with data url

Status: VERIFIED FIXED

(Firefox :: Page Info Window, defect)

Description

[Emil Ivanov]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008032904 Minefield/3.0pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008032904 Minefield/3.0pre

There is no way to preview image or background when its displayed with data url.

Reproducible: Always

Comment 2

[Emil Ivanov]

As image in img tags are seen in fx2 which is mentioned in bug 386333
They are not in fx3

Comment 3

[Florian Quèze [:florian]]

Attachment: patch v1

I guess this is a regression from bug 377364.

Comment 4

[Emil Ivanov]

I tested only one build for now
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070101 Minefield/3.0a2pre
object tag containing encoded image is displayed in media preview on the acid2 test.

Comment 5

[Florian Quèze [:florian]]

Attachment: patch v2 (includes the fix for background images with data url)

If we want to fix the case of background images in data urls (bug 386333) at the same time, we can add:

  // if we have a data url, get the MIME type from the url
  if (!mimeType && /^data:/.test(url)) {
    var dataMimeType = /^data:(image\/.*);/.exec(url);
    if (dataMimeType)
      mimeType = dataMimeType[1];
  }

I'm not sure if there is any security implication though. Currently, we do this test (/^data:/.test(url) && /^image\//.test(mimeType)) before allowing the display of an image from a data URL. If we take into account the MIME type included in the URL, the test won't be as reliable as before, because the MIME type included in the URL can be different from the actual type of the image.

I don't remember why we added the /^image\//.test(mimeType) test so I don't know if this is a real issue.

Comment 6

[Asaf Romano (gone)]

Comment on attachment 312606 [details] [diff] [review]
patch v1

r=mano

Comment 8

[Dão Gottwald [:dao]]

landed v1:

http://hg.mozilla.org/mozilla-central/rev/818c6feeb5e9

Comment 9

[Dão Gottwald [:dao]]

If someone wants to see this in 3.5, you need to request "approval1.9.1" on the patch.

Comment 10

[Emil Ivanov]

(In reply to comment #9)
> If someone wants to see this in 3.5, you need to request "approval1.9.1" on the
> patch.

And what about v2?

Comment 11

[Florian Quèze [:florian]]

(In reply to comment #10)
> (In reply to comment #9)
> > If someone wants to see this in 3.5, you need to request "approval1.9.1" on the
> > patch.
> 
> And what about v2?

Can be covered in bug 386333. This (the difference between v1 and 2) may help: https://bugzilla.mozilla.org/attachment.cgi?oldid=312606&newid=312608&action=interdiff&format=raw

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Florian, will you move the patch to bug 386333 and ask for review there?

Comment 13

[Florian Quèze [:florian]]

No, or at least, not in the near future.  If you understand the patch and are interested, feel free to do it.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #13)
> No, or at least, not in the near future.  If you understand the patch and are
> interested, feel free to do it.

Thanks Forian. I've attached the updated patch.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090407 Minefield/3.6a1pre (.NET CLR 3.5.30729) ID:20090407095250 and build on OS X.

Comment 16

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 312606 [details] [diff] [review]
patch v1

a191=beltzner

Comment 17

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/89bf2fbc175c

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

While verifying this patch on 1.9.1 I noticed that Shiretoko displays some more media references as Minefield. Most of them are still broken. Does it mean we fail even with this patch on 1.9.1 or are those bogus entries?

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #18)
> While verifying this patch on 1.9.1 I noticed that Shiretoko displays some more
> media references as Minefield. Most of them are still broken. Does it mean we
> fail even with this patch on 1.9.1 or are those bogus entries?

This should be another issue. No time right now to dig into this issue.

Verified fixed on 1.9.1 with builds on OS X and Windows:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090423 Shiretoko/3.5b4pre ID:20090423040926