1048579 - Ignore Content-Type header when determining media types

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

See https://www.w3.org/Bugs/Public/show_bug.cgi?id=11984

Now that IE has changed it's going to become essential for Web compatibility, so we should do this as soon as possible. It should be a small change.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

I actually think this is all that's required --- plus appropriate test changes.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 3

[Boris Zbarsky [:bzbarsky]]

This patch changes the behavior of something like:

  <iframe src="some-uri">

where some-uri returns "Content-Type: text/html" but data that nsMediaSniffer::GetMIMETypeFromContent thinks is media data.  The old code would treat it as HTML, but the new code will treat it as media.

That is, this is changing the behavior of loads in non-media contexts...  Is that actually desired?  Do other UAs do that?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Chrome doesn't seem to load media in http://people.mozilla.org/~roc/media-iframe.html. I'm not sure what it does, it just loads forever.

What I should I do here? Add a new nsIChannel flag just for media loads?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> I'm not sure what it does, it just loads forever.

Maybe trying to render the 37MB of random data as HTML?  ;)

We already had an nsIChannel flag just for media loads: LOAD_TREAT_APPLICATION_OCTET_STREAM_AS_UNKNOWN.  We could just rename it to LOAD_ALLOW_SNIFFING_MEDIA or something and rip out the necko bits, then check for it in the media sniffer.  A quick mxr glance suggests that addons don't use it, which is not surprising.

Another option is to just set something in the channel property bag, or the channel's LoadInfo, that identifies media loads, and then check it in the media sniffer.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Boriz Zbarsky [:bz] from comment #5)
> > I'm not sure what it does, it just loads forever.
> 
> Maybe trying to render the 37MB of random data as HTML?  ;)

Gecko renders it just fine :-)

> We already had an nsIChannel flag just for media loads:
> LOAD_TREAT_APPLICATION_OCTET_STREAM_AS_UNKNOWN.  We could just rename it to
> LOAD_ALLOW_SNIFFING_MEDIA or something and rip out the necko bits, then
> check for it in the media sniffer.  A quick mxr glance suggests that addons
> don't use it, which is not surprising.

OK I'll do that.

Comment 7

[Boris Zbarsky [:bzbarsky]]

> Gecko renders it just fine :-)

YMMV.  Mine hung for about 2-3 minuts, then I killed it.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Seems to work based on cursory testing.

https://tbpl.mozilla.org/?tree=Try&rev=0dbeb8cd677b

Note that all I've done here is relax the media sniffer so for media element loads, it can override an existing content type. I think this makes sense as long as our sniffers accept disjoint sets of resources.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8476485 [details] [diff] [review]
fix

>+     * If this flag is set, the server's Content-Type will be completely ignored
>+     * and only sniffing will be used to determine the Content-Type.

I don't believe this is true: the server's type will be honored, unless we detect that the data is media data.

r=me with this comment fixed.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Boriz Zbarsky [:bz] from comment #9)
> >+     * If this flag is set, the server's Content-Type will be completely ignored
> >+     * and only sniffing will be used to determine the Content-Type.
> 
> I don't believe this is true: the server's type will be honored, unless we
> detect that the data is media data.

Yeah I forgot to update the comment.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://tbpl.mozilla.org/?tree=Try&rev=2b7377c2bf76

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ecf04c6eb4dd

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Backed out for xpcshell failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f72335461979

https://tbpl.mozilla.org/php/getParsedLog.php?id=46557589&tree=Mozilla-Inbound

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is a bit mysterious. Tests pass for me locally on Linux, both 32-bit debug and 64-bit opt. Try build with some logging:
https://tbpl.mozilla.org/?tree=Try&rev=670bbebfe178

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't get it. This is another push of that patch on new-ish mozilla-inbound:
https://tbpl.mozilla.org/?tree=Try&rev=5592d0a75852
It has slightly different logging compiled in, but that's all, and it passed. I'll try relanding it.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/35899dd74748

Comment 17

[Ryan VanderMeulen [:RyanVM]]

You didn't bump the UUID with your IDL change.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Feeling nice today, so bumping the UUIDs rather than backing out and clobbering all over again. Please keep in mind for the future that IDL changes don't get noticed by the build system unless the UUID is bumped. Try builds always clobber which is why you didn't see that bustage there.
https://hg.mozilla.org/integration/mozilla-inbound/rev/822c7f854920

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/35899dd74748
https://hg.mozilla.org/mozilla-central/rev/822c7f854920

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #18)
> Feeling nice today, so bumping the UUIDs rather than backing out and
> clobbering all over again. Please keep in mind for the future that IDL
> changes don't get noticed by the build system unless the UUID is bumped. Try
> builds always clobber which is why you didn't see that bustage there.
> https://hg.mozilla.org/integration/mozilla-inbound/rev/822c7f854920

Thanks!!!

Comment 21

[Tony Chung [:tchung]]

Qanalysts, please test if this is working and fixed against 2.1 and master.  thanks!

Comment 22

[Joshua Mitchell (Inactive)]

Flame device was not able to Play M4V files, Video app was not able to detect M4V files.

Using the following STR

1) in the search bar type in M4V 
2) Click on the Google Icon
3) Type in 'Sample' into the google search bar behind m4v
4) Click the 2nd link and download the sample
5) Click the Download Complete Message

Actual results - Video is not opened - "Unable to open" error message is given.

6) Proceed to Settings > Downloads 
7) Select file > Open

Same Results

8) Open Video Player

Actual Results - Video is not present

--------------------------------------------------------------------------------------

Tested on v180 Base with latest 2.1 and 2.2 (tinderbox) build shallow-flashed; With just the V184 Base, and then with 2.2 (Nightly) Full-flashed on top of that


Device: Flame Master
Build ID: 20141008065408
Gaia: 2665e714beea5dc433862ca6bb8d2b47ffe2f2d1
Gecko: 4bad24a306b2
Version: 35.0a1 (Master)
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Environmental Variables:
Device: Flame 2.1
Build ID: 20141007075118
Gaia: da328c6cbabf2cffc2d362e282cacc93325d1f43
Gecko: aebe54593d60
Version: 34.0a2 
Firmware Version: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Tested on the KK V184 BASE ONLY

Device: Flame 2.0 (Base Only)
Build ID: 20140930142714
Gaia: 1dc2e29491da234cfa461916304d77ce88b50045
Gecko: 43a3a6cc35be963f5c987b6d121e8d14a593e9df
Version: 32.0 (2.0)
Firmware Version: V184
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame Master
Build ID: 20141008065430
Gaia: 0bc74ce502672cf0265b24cf3a25d117c3de5e71
Gecko: dd7637cc42d5
Version: 35.0a1 (Master)
Firmware Version: V184
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Comment 23

[Joshua Mitchell (Inactive)]

Attachment: Video Sample 1 (Small).m4v

Sample M4V file downloaded

Comment 24

[Joshua Mitchell (Inactive)]

Attachment: logcat_20141008_1330.txt

Logcat ran as I go to Downloads in settings and select M4V file

Comment 25

[Tony Chung [:tchung]]

This is working for me.   I tried streaming the test file in thie bug, both through the browser, and downloading and playing it locally.   My screencast of it working:  http://youtu.be/QQLzxPrv-dM

I'm on a Flame KK v184 image, with full flash 2.1 over it.

Gaia-Rev        d71f8804d7229f4b354259d5d8543c25b4796064
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-aurora/rev/7fa82c9acdf2
Build-ID        20141008000201
Version         34.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141008.034603
FW-Date         Wed Oct  8 03:46:14 EDT 2014
Bootloader      L1TC00011840

I've asked Joshua to try again on another device.

Comment 26

[Joshua Mitchell (Inactive)]

I tested on another Flame - I AM able to play the file through the browser on either device (by going to this bugzilla page and selecting the file I uploaded to this bug)
I am also able to play the file by long pressing on the file and downloading it from the bugzilla page

but am NOT able to play the file when downloading it from the original source - 2nd result from googling 'm4v sample' 

Tested on Flame KK V184 image, with full flash 2.1 over it.

Device: Flame 2.1
Build ID: 20141008000201
Gaia: d71f8804d7229f4b354259d5d8543c25b4796064
Gecko: 7fa82c9acdf2
Version: 34.0a2 (Master)
Firmware Version: V184
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 27

[Tony Chung [:tchung]]

(In reply to Joshua Mitchell [:Joshua_M] from comment #26)
> but am NOT able to play the file when downloading it from the original
> source - 2nd result from googling 'm4v sample' 
> 
I'm going to assume there's a problem with the source host, or decoding the file on the fly when you are googling it.   Does the same behavior happen on Desktop?

Given the previous two comments, marking this bug as verified on 2.1 branch for loading of m4v container types.

Comment 28

[Joshua Mitchell (Inactive)]

(In reply to Tony Chung [:tchung] from comment #27)
> I'm going to assume there's a problem with the source host, or decoding the
> file on the fly when you are googling it.   Does the same behavior happen on
> Desktop?
> 

It works fine on the Desktop - I can open the file or save it and open it.