1720353 - Implement navigator.pdfViewerEnabled / hard-coded navigator.plugins + navigator.mimeTypes

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, task)

Description

[Domenic Denicola]

• Spec PR: https://github.com/whatwg/html/pull/6738
• New spec: https://html.spec.whatwg.org/multipage/system-state.html#pdf-viewing-support
• Tests: https://github.com/web-platform-tests/wpt/pull/29559 (not yet reviewed/merged)

This obsoletes https://bugzilla.mozilla.org/show_bug.cgi?id=1687845 and would fix https://bugzilla.mozilla.org/show_bug.cgi?id=840439

Comment 1

[Anne (:annevk)]

This has successfully shipped in Chrome. We should consider aligning here.

David, is this something your team can take on?

Comment 2

[David Parks [:handyman]]

will do

Comment 3

[David Parks [:handyman]]

I'm wrapping this up but I don't know our approach around web-compatibility tests. The test here has everything I'd need for testing but I don't know when it is run in automation or if that would be considered sufficient. Do you know if we usually write e.g. mochitests that cover the same things as those tests, or do something else?

Also, in my try push, I'm getting that the test is failing because it is an unexpected-pass with my changes:

[task 2021-12-07T22:06:48.006Z] 22:06:48 INFO - TEST-UNEXPECTED-PASS | /html/webappapis/system-state-and-capabilities/the-navigator-object/plugins-and-mimetypes.html | navigator.pdfViewerEnabled exists - expected FAIL

Where do I update this so it understands that we should pass now?

Comment 4

[Anne (:annevk)]

Hey David, great to hear this is close to being finished! web-platform-tests are run in automation and I'd generally encourage them over mochitests when possible as they are shared with other browsers (making it more likely we all implement the same thing). In this case it seems all that remains to do here for you is to remove https://searchfox.org/mozilla-central/source/testing/web-platform/meta/html/webappapis/system-state-and-capabilities/the-navigator-object/plugins-and-mimetypes.html.ini (the default expectation is PASS). \o/

Comment 5

[David Parks [:handyman]]

Attachment: Bug 1720353: Implement new navigator.{plugins,mimeTypes,pdfViewerSupported} specs r=peterv!

This implements the new HTML spec for these fields, which now serve hard-coded values, depending on whether or not PDFs are supported. The values were deemed important to maintain web compatibility. The spec can be found in section 8.9.1.6:

https://html.spec.whatwg.org/multipage/system-state.html#pdf-viewing-support

The web-compat test for this can be found at:

https://wpt.live/html/webappapis/system-state-and-capabilities/the-navigator-object/plugins-and-mimetypes.html

This patch follows the spec for the PDF plugins if "pdfjs.disabled" is false. It also produces empty plugin arrays if "pdfjs.disabled" is true, as per the spec. Both cases are tested by the wpt.live page.

Comment 6

[David Parks [:handyman]]

(In reply to Anne (:annevk) from comment #4)

Hey David, great to hear this is close to being finished! web-platform-tests are run in automation and I'd generally encourage them over mochitests when possible as they are shared with other browsers (making it more likely we all implement the same thing). In this case it seems all that remains to do here for you is to remove https://searchfox.org/mozilla-central/source/testing/web-platform/meta/html/webappapis/system-state-and-capabilities/the-navigator-object/plugins-and-mimetypes.html.ini (the default expectation is PASS). \o/

I'd swear I searched for this like a dozen times. :-P

https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=UklCUfJ3TD6qrdXR58Zx6Q.0&revision=154c383bd68afff16419bdb6ccf51f3689069730

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fcbba2f8f8c
Implement new navigator.{plugins,mimeTypes,pdfViewerSupported} specs r=peterv

Comment 8

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on test_bug1281963.html

• backout: https://hg.mozilla.org/integration/autoland/rev/95970cc3e5a3a5c1b74b36cbdd192861e23801f2
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=faGwb7QiR_aR_PO9BCQWew.0&revision=5fcbba2f8f8cab30a1f86b7ae24a724db5fcf8d5
• failure log: https://treeherder.mozilla.org/logviewer?job_id=367432515&repo=autoland&lineNumber=1930

[task 2022-02-10T17:32:59.530Z] 17:32:59     INFO -  TEST-PASS | dom/base/test/test_bug1281963.html | navigator.plugins.length should be nonzero
[task 2022-02-10T17:32:59.531Z] 17:32:59     INFO -  Buffered messages finished
[task 2022-02-10T17:32:59.531Z] 17:32:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1281963.html | Don't reveal mime type - got [object MimeType], expected undefined
[task 2022-02-10T17:32:59.531Z] 17:32:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-02-10T17:32:59.531Z] 17:32:59     INFO -      @dom/base/test/test_bug1281963.html:51:7
[task 2022-02-10T17:32:59.532Z] 17:32:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1281963.html | Don't reveal mime type - got [object MimeType], expected undefined
[task 2022-02-10T17:32:59.532Z] 17:32:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-02-10T17:32:59.532Z] 17:32:59     INFO -      @dom/base/test/test_bug1281963.html:53:5
[task 2022-02-10T17:32:59.532Z] 17:32:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1281963.html | navigator.mimeTypes.length should be 0 - got 2, expected +0
[task 2022-02-10T17:32:59.532Z] 17:32:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-02-10T17:32:59.532Z] 17:32:59     INFO -      @dom/base/test/test_bug1281963.html:54:5
[task 2022-02-10T17:32:59.532Z] 17:32:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1281963.html | Don't reveal plugin - got [object Plugin], expected undefined
[task 2022-02-10T17:32:59.532Z] 17:32:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-02-10T17:32:59.533Z] 17:32:59     INFO -      @dom/base/test/test_bug1281963.html:56:7
[task 2022-02-10T17:32:59.533Z] 17:32:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1281963.html | Don't reveal plugin - got [object Plugin], expected undefined
[task 2022-02-10T17:32:59.533Z] 17:32:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-02-10T17:32:59.533Z] 17:32:59     INFO -      @dom/base/test/test_bug1281963.html:58:5
[task 2022-02-10T17:32:59.533Z] 17:32:59  WARNING -  TEST-UNEXPECTED-FAIL | dom/base/test/test_bug1281963.html | navigator.plugins.length should be 0 - got 5, expected +0
[task 2022-02-10T17:32:59.533Z] 17:32:59     INFO -      SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-02-10T17:32:59.534Z] 17:32:59     INFO -      @dom/base/test/test_bug1281963.html:59:5
[task 2022-02-10T17:32:59.534Z] 17:32:59     INFO -  add_task | Leaving test
[task 2022-02-10T17:32:59.534Z] 17:32:59     INFO -  TEST-OK | dom/base/test/test_bug1281963.html | took 60ms

Comment 9

[Natalia Csoregi [:nataliaCs]]

Also failing: https://treeherder.mozilla.org/logviewer?job_id=367427739&repo=autoland&lineNumber=5791

Comment 10

[David Parks [:handyman]]

Attachment: Bug 1720353: Make navigator test expect plugins when resistFingerprinting is not set r=#preferences-reviewers!

resistFingerprinting used to be set for the whole test but was changed to consider cases where it is unset. In those cases, pluginsLength and mimeTypesLength tests were only passing because there were no plugins installed and, later, no plugins were permitted. We now hardcode some fixed PDF entries so the test has to consider that.

Depends on D133291

Comment 11

[David Parks [:handyman]]

It just needed to consider privacy.resistFingerprinting the same way it used pdfjs.disabled. As for the second test, the test has been coincidentally passing because the entries were empty but, now that they aren't, it needed to be updated.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3593056496d
Implement new navigator.{plugins,mimeTypes,pdfViewerSupported} specs r=peterv
https://hg.mozilla.org/integration/autoland/rev/d02b8df13ab0
Make navigator test expect plugins when resistFingerprinting is not set r=preferences-reviewers,Gijs

Comment 13

[Cristian Tuns]

Backed out for causing reftest failures on 1113005.html

• Backout link
• Push with failures
• Failure Log
• Failure line: REFTEST PROCESS-CRASH | xpcom/string/crashtests/1113005.html (finished) | application crashed [@ mozilla::LinkedList<nsSHistory>::~LinkedList()]

Comment 14

[David Parks [:handyman]]

Not really reftest, the issue was a missing cycle collector addition: https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=dwIIpbwHTXiSTGGdTmjvEQ.0&revision=ac93bb2cb11eea9c74e2ba3ca75b05b30ba35db3

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33150ccd2252
Implement new navigator.{plugins,mimeTypes,pdfViewerSupported} specs r=peterv
https://hg.mozilla.org/integration/autoland/rev/6f35bc824b37
Make navigator test expect plugins when resistFingerprinting is not set r=preferences-reviewers,Gijs

Comment 16

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/33150ccd2252
https://hg.mozilla.org/mozilla-central/rev/6f35bc824b37

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/f40a587348f8
Port bug 1720353 - Remove "pdfjs.disabled" from all-thunderbird.js. rs=linting DONTBUILD

Comment 18

[Hamish Willee]

FF98 docs work for this can be tracked in https://github.com/mdn/content/issues/13370.

I'm finding the spec terminology and big thread confusing. Would be great if you could help confirm a few points.

navigator.pdfViewerEnabled - > Returns true if the user agent supports inline viewing of PDF files when navigating to them, or false otherwise. In the latter case, PDF files will be handled by external software.

1. Would I be correct in taking this to mean: true if PDF can be displayed inline (using a plugin or internal viewer), and false otherwise. If false, it will be downloaded rather than displayed inline on navigation

   • so handled "by external software" means "download" - the browser doesn't care any more.

2. My understanding is that this is intended to replace using Navigator.mimeTypes and Navigator.plugins to work out if inline PDFs are supported. The problem with that was that people were assuming a few known strings for plugins/inline viewers, which:
   a) sent an incorrect signal about what was supported if the set didn't match
   b) enabled fingerprinting from the plugin information.

So what you have done is also updated the (deprecated) Navigator.mimeTypes so that if inline browsing is not supported it returns an empty array.
If inline browsing is supported it supplies a set of 5 plugins that most websites do their PDF checks against.
So in summary, things that haven't updated to navigator.pdfViewerEnabled get the same signal from the legacy API, without info that can be used for fingerprinting.

3. Is ^^^ correct?

4. A bit that is not clear to me is what you can do with the array of plugins. I.e. when on FF a user will still see the chrome plugin, which is not actually present. Can this actually ever be enabled? And in that case what happens? My guess is that either it can't be enabled (there is no API for that)

5. The Navigator.mimeTypes and plugins are deprecated. My understanding is that they remain deprecated - right?

6. What set of plugins and mimeTypes might have been returned before for plugins? My assumption is pretty much none and so that changing these just to the PDF plugins has zero impact. Right?

Comment 19

[David Parks [:handyman]]

(In reply to Hamish Willee from comment #18)

1. Would I be correct in taking this to mean: true if PDF can be displayed inline (using a plugin or internal viewer), and false otherwise. If false, it will be downloaded rather than displayed inline on navigation
   • so handled "by external software" means "download" - the browser doesn't care any more.

Correct.

2. My understanding is that this is intended to replace using Navigator.mimeTypes and Navigator.plugins to work out if inline PDFs are supported. The problem with that was that people were assuming a few known strings for plugins/inline viewers, which:
   a) sent an incorrect signal about what was supported if the set didn't match
   b) enabled fingerprinting from the plugin information.

Also correct.

So what you have done is also updated the (deprecated) Navigator.mimeTypes so that if inline browsing is not supported it returns an empty array.
If inline browsing is supported it supplies a set of 5 plugins that most websites do their PDF checks against.
So in summary, things that haven't updated to navigator.pdfViewerEnabled get the same signal from the legacy API, without info that can be used for fingerprinting.

Correct.

4. A bit that is not clear to me is what you can do with the array of plugins. I.e. when on FF a user will still see the chrome plugin, which is not actually present. Can this actually ever be enabled? And in that case what happens? My guess is that either it can't be enabled (there is no API for that)

No, there's no concept of enabling for these - they are just strings. They come from the HTML spec .

5. The Navigator.mimeTypes and plugins are deprecated. My understanding is that they remain deprecated - right?

Officially? I don't know that. But they are coded to fixed strings with no real meaning beyond "A PDF Viewer Is Supported".

6. What set of plugins and mimeTypes might have been returned before for plugins? My assumption is pretty much none and so that changing these just to the PDF plugins has zero impact. Right?

Previously the list included Adobe Flash, if present. IIRC it didn't have any entries for the two ever-present ones mentioned in about:plugins.

Comment 20

[Hamish Willee]

Thanks very much - that is very helpful