Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only support the standard, unprefixed, Fullscreen API in the default viewer #14606

Merged
merged 2 commits into from
Feb 26, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

At this point in time, after recent rounds of clean-up, the webkit-prefixed Fullscreen API is the only remaining browser-specific compatibility hack in the web/-folder JavaScript code.

The standard, and thus unprefixed, Fullscreen API has been supported for over three years in both Mozilla Firefox and Google Chrome. According to MDN, the unprefixed Fullscreen API has been available since:

Hence only Safari now requires using a prefixed Fullscreen API, and it's thus (significantly) lagging behind other browsers in this regard.
Considering that the default viewer is written specifically to be the UI for the Firefox PDF Viewer, and that we ask users to not just use it as-is[1], I think that we should only support the standard Fullscreen API now.
Furthermore, note also that the FAQ already lists Safari as "Mostly" supported; see https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support


[1] Note e.g. http://mozilla.github.io/pdf.js/getting_started/#introduction

The viewer is built on the display layer and is the UI for PDF viewer in Firefox and the other browser extensions within the project. It can be a good starting point for building your own viewer. However, we do ask if you plan to embed the viewer in your own site, that it not just be an unmodified version. Please re-skin it or build upon it.

Now that the there's ECMAScript support for properly private methods on `class`es, we can use that instead and thus remove all of the `@private` JSDocs comments.
…viewer

At this point in time, after recent rounds of clean-up, the `webkit`-prefixed Fullscreen API is the only remaining *browser-specific* compatibility hack in the `web/`-folder JavaScript code.

The standard, and thus unprefixed, Fullscreen API has been supported for *over three years* in both Mozilla Firefox and Google Chrome. [According to MDN](https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API#browser_compatibility), the unprefixed Fullscreen API has been available since:
 - Mozilla Firefox 64, released on 2018-12-11; see https://wiki.mozilla.org/Release_Management/Calendar#Past_branch_dates
 - Google Chrome 71, released on 2018-12-04; see https://en.wikipedia.org/wiki/Google_Chrome_version_history

Hence *only* Safari now requires using a prefixed Fullscreen API, and it's thus (significantly) lagging behind other browsers in this regard.
Considering that the default viewer is written *specifically* to be the UI for the Firefox PDF Viewer, and that we ask users to not just use it as-is[1], I think that we should only support the standard Fullscreen API now.
Furthermore, note also that the FAQ already lists Safari as "Mostly" supported; see https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#faq-support

---
[1] Note e.g. http://mozilla.github.io/pdf.js/getting_started/#introduction
> The viewer is built on the display layer and is the UI for PDF viewer in Firefox and the other browser extensions within the project. It can be a good starting point for building your own viewer. *However, we do ask if you plan to embed the viewer in your own site, that it not just be an unmodified version. Please re-skin it or build upon it.*
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/fefeec62f62136f/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/fefeec62f62136f/output.txt

Total script time: 2.82 mins

Published

@timvandermeij timvandermeij merged commit 620174a into mozilla:master Feb 26, 2022
@timvandermeij
Copy link
Contributor

I agree; thank you for doing this!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants