requestAnimationFrame should not monkey patch over window

[Tyler-V]

pdf.js anonymously monkey patches over the window.requestAnimationFrame() namespace which doesn't allow the user to unpatch. Register a function with a new name on the window (i.e. Window.prototype.requestAnimFrame) or use a browser patch to check if raf is supported before replacing it with a setTimeout().

Configuration:

• Web browser and its version: Chrome: Version 61.0.3163.79 (Official Build) (64-bit)
• Operating system and its version: Windows 10
• PDF.js version: ^1.2.5
• Is an extension: ng2-pdf-viewer

What is the expected behavior? To be able to call window.requestAnimationFrame() elsewhere in an application and not have it hijacked by pdf.js's raf patch.

What went wrong?

                (function checkRequestAnimationFrame() {
                    function installFakeAnimationFrameFunctions() {
                        window.requestAnimationFrame = function(callback) {
                            return window.setTimeout(callback, 20);
                        };            

installFakeAnimationFrameFunctions creates a bad shim for smooth animations, affecting the user experience throughout the application.

My suggestion would be to use Paul Irish's polyfill (which is widely accepted/used)
https://www.paulirish.com/2011/requestanimationframe-for-smart-animating/

[Snuffleupagus]

or use a browser patch to check if raf is supported before replacing it with a setTimeout().

But we do check if requestAnimationFrame is supported before adding the polyfill, please see in particular:

pdf.js/src/shared/compatibility.js

Lines 727 to 735 in d0d7046

	if ('requestAnimationFrame' in window) {
	return;
	}
	window.requestAnimationFrame = window.mozRequestAnimationFrame ||
	window.webkitRequestAnimationFrame;
	if (window.requestAnimationFrame) {
	return;
	}
	installFakeAnimationFrameFunctions();

Register a function with a new name on the window (i.e. Window.prototype.requestAnimFrame)

Considering that all modern browsers already have requestAnimationFrame support, according to https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame#Browser_compatibility, we thus don't want to modify the source code just to accommodate the polyfill (which isn't even included in e.g. the Firefox/Chrome extensions).

PDF.js version: ^1.2.5

That version of PDF.js is first of all several years old, and secondly it doesn't match any official release as listed in https://github.com/mozilla/pdf.js/releases.

Is an extension: ng2-pdf-viewer

The "Is an extension" question refers to https://github.com/mozilla/pdf.js#browser-extensions. Please note that we cannot support third-party libraries that depend on PDF.js, hence you'll need to ask for support directly from its maintainer.

To be able to call window.requestAnimationFrame() elsewhere in an application and not have it hijacked by pdf.js's raf patch.

As indicated above, PDF.js will not replace the native requestAnimationFrame function nor an already (by other code outside of PDF.js) registered polyfill.

In summary, I'm not convinced that we need/should make any changes here.

[timvandermeij]

I'm closing this as answered by the comment above. I too see nothing else that should be done here on the PDF.js side. Make sure to use the most recent version of PDF.js since we do check if RAF is supported before polyfilling it.

[txinfo]

@timvandermeij @Snuffleupagus

Actually on iOS it appears a polyfill is forced. Can you consider reopening this issue?

if (isIOS) { // requestAnimationFrame on iOS is broken, replacing with fake one. installFakeAnimationFrameFunctions(); return; }

from

pdf.js/src/shared/compatibility.js

Lines 571 to 575 in 6521d2f

	if (isIOS) {
	// requestAnimationFrame on iOS is broken, replacing with fake one.
	installFakeAnimationFrameFunctions();
	return;
	}

[yurydelendik]

Can you consider reopening this issue?

Opened project/proposal at https://github.com/mozilla/pdf.js/projects/6

[Snuffleupagus]

Actually on iOS it appears a polyfill is forced. Can you consider reopening this issue?

Considering that the requestAnimationFrame polyfill was removed in PR #9138 (in preparation for PDF.js version 2.0), I'm not sure what purpose re-opening this issue would serve!?

[txinfo]

Yes the problem still exists only on the 1.x branch (another library uses pdf.js 1.x as it's dependency which is where I encountered the issue originally). so assuming that branch is no longer maintained this ticket can remain closed.