1499169 - embed/object "application/pdf" is not rendered with "charset=UTF-8" added in type

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[thywalls]

Attachment: pdfTest.html

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:62.0) Gecko/20100101 Firefox/62.0

Steps to reproduce:

I detected in a local goverment website, a pdf doesn't render as always did. The line about the pdf is:

<object data="pdfTest.pdf" type="application/pdf;charset=UTF-8" width="90%" height="600"></object>


Actual results:

The pdf is not rendered in the page with pdfjs in Firefox 62.0.3. Tested in different computers.

Also it doesn't render using the line:

<embed src="pdfTest.pdf" width="90%" height="600" type="application/pdf;charset=UTF-8"></embed>


Expected results:

The pdf appears embedded in the page with pdfjs, like does in IE, Chrome (lastest), and older Firefox like Fx 52.9 ESR.

I see if you put only 'type="application/pdf"' instead of 'type="application/pdf;charset=UTF-8"', it works in Fx 62.0.3.

<embed src="pdfTest.pdf" width="90%" height="600" type="application/pdf"></embed>
<br />
<object data="pdfTest.pdf" type="application/pdf" width="90%" height="600"></object>

Both works.

Comment 1

[thywalls]

Attachment: pdfTest.pdf

Comment 2

[thywalls]

Maybe is a new standart limitation or security measurement, but i didn't find any information.

I attached a html and a pdf, the html doesn't show any pdf, as does all other browsers (and Firefox older versions).

Comment 3

[Gingerbread Man]

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=69862af7f7d231976331783f0c4711bcb04466ce&tochange=e5f69bca3ee6fd5e67ab876db7d220acc26ab75c

(In reply to thywalls from comment #0)
> The pdf appears embedded in the page with pdfjs, like does in IE

IE11 doesn't display anything for me. Edge does though.

(In reply to thywalls from comment #2)
> Maybe is a new standart limitation or security measurement, but i didn't
> find any information.

The mozregression-gui tool can reveal which specific bug introduced an unwanted change. In this case, bug 1473833.
https://mozilla.github.io/mozregression/quickstart.html

Comment 4

[thywalls]

(In reply to Gingerbread Man from comment #3)
> (In reply to thywalls from comment #0)
> > The pdf appears embedded in the page with pdfjs, like does in IE
> 
> IE11 doesn't display anything for me. Edge does though.

Yes, it's true. In IE11 appears an empty box with embed, but nothing with object. But i think it is because IE11 hasn't anything to preview pdfs, because if you have installed Adobe Reader Plugin in IE, with both appears the pdf preview. I only want to point that out.

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Huh, not sure what charsets in MIME types would trip up our detection, as application/pdf should be specially routed to pdf/js. I'll check this out.

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

So the problem here is that charset isn't valid for application/pdf (or at least, there's nothing listed in https://tools.ietf.org/html/rfc3778). charset is used for text/*, application/json, and other text formats, but application/pdf is a binary format, so the charset addition will be ignored.

Unfortunately, almost all of the MIME checks in our codebase simply test against a "application/pdf" string, so the addition of the charset causes those to fail. That means the only reason that "application/pdf;charset=utf-8" worked in the first place was due to our fluke of rendering wrong MIME types as documents in the first place anyways, which Bug 1473833 fixed.

:bz, got any thoughts on this? I suppose I could add some sort of "IsPDFMIMEType" function to ContentUtils or something that does the extra work using MimeType or whatever to figure out whether we're getting application/pdf;charset=whatever, even if we don't use any of the Mime Parameters? Just asking 'cause this is one of those webcompat over what the spec says situations.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Well, if the spec isn't webcompat here, we should presumably get the spec fixed.  That said, the spec does say that the "type" attribute of <object> is https://mimesniff.spec.whatwg.org/#valid-mime-type which can include parameters, and the actual handling of the "type" attr for object is ... not that clearly specified.

In terms of implementation, you could nsContentUtils::SplitMimeType.  That's what we use for <link type="..."> and matching <source> elements and whatnot.

Comment 8

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Bug 1499169 - Split MIME type of nsObjectLoadingContent when retreived from tag;

Adds a nsContentUtils method to check for pdf mime types, ignoring
parameters that may be included with the type.

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Bug 1499169 - Split MIME type of nsObjectLoadingContent when retreived from tag;

MIME type should be trimmed of parameters whenever it is retreived
from the tag it relates to, otherwise other checks may fail.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f6f47d5b05b6
Split MIME type of nsObjectLoadingContent when retreived from tag; r=bzbarsky

Comment 11

[Cristian Brindusan [:cbrindusan]]

Backed out changeset f6f47d5b05b6 (Bug 1499169) for mochitest failures on test_bug1499139.html.

Backout: https://hg.mozilla.org/integration/autoland/rev/0d21fdb4886c98608edce9b79fc6927baf2b4f05

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f6f47d5b05b68597739e059cc707573a3d1cea01&selectedJob=209943711

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=209943711&repo=autoland&lineNumber=1434

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/86f720f7f3a6
Split MIME type of nsObjectLoadingContent when retreived from tag; r=bzbarsky

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/86f720f7f3a6

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance.

Comment 15

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 9022698 [details]
Bug 1499169 - Split MIME type of nsObjectLoadingContent when retreived from tag;

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1473833

User impact if declined: Some object tags with parameters may not load contents even though mime type is mostly valid

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Just refines how we parse mime types, mostly a bug fix.

String changes made/needed: None

Comment 16

[Julien Cristau [:jcristau]]

Comment on attachment 9022698 [details]
Bug 1499169 - Split MIME type of nsObjectLoadingContent when retreived from tag;

regression fix, with test, approved for 64.0b8

Comment 17

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cd20a71aba8a

Comment 18

[Kanchan Kumari QA]

Does this need manual qa? If yes can you please provide STR, so that I can verify the fix on latest Beta and Nightly. Thanks!

Comment 19

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Nah, automated test covers it.

Comment 20

[Kanchan Kumari QA]

Thanks Kyle! Based on comment 19, I am changing the qe verify flag.