Enable range requests in Safari (both OSX and IOS)

[EugeneSqr]

Attach (recommended) or Link to PDF file here: https://mozilla.github.io/pdf.js/web/compressed.tracemonkey-pldi-09.pdf

Configuration:

• Web browser and its version: Safari 11.1 (OSX), Safari 11.0 (IOS)
• Operating system and its version: OSX 10.13.4, IOS 11.4
• PDF.js version: latest from master
• Is a browser extension: no

Steps to reproduce the problem:

1. Start dev server with gulp server
2. Visit http://localhost:8888/web/viewer.html
3. Observe network tab for 206 requests

What is the expected behavior? (add screenshot)
The document is loaded using range requests

What went wrong? (add screenshot)
The document is loaded with a single request. It looks like range requests were explicitly disabled by this code here:

  // Support: Safari 6.0+, iOS
  (function checkRangeRequests() {
    // Safari has issues with cached range requests, see issue #3260.
    // Last tested with version 6.0.4.
    if (isSafari || isIOS) {
      compatibilityParams.disableRange = true;
      compatibilityParams.disableStream = true;
    }
  })(); 

It feels a bit outdated. I've just tested pdfjs without this code in the specified browsers and it worked just fine. Is there a reason for keeping this code?

Link to a viewer (if hosted on a site other than mozilla.github.io/pdf.js or as Firefox/Chrome extension):

[timvandermeij]

Not really, and we would like to limit it where possible. Could you indicate from which Safari/iOS versions range request work again, since it was apparently broken before?

[yurydelendik]

@EugeneSqr are you familiar with related bug reports, e.g. #3260. Looks like https://bugs.webkit.org/show_bug.cgi?id=144682 addresses that in WK2 (after 2018-1-3).

[fooware]

To me it looks like the 2018-01-03 update only affected the test cases and the actual fix was merged around 2015-05. This is also collaborated by comments in this bug report that dupes it:
https://bugs.webkit.org/show_bug.cgi?id=82672

In that bug report you can also find an interesting dynamic workaround for issue in another project:
bvibber/ogv.js@9ab5bce

[timvandermeij]

For PDF.js 2.0 we only support Safari 8+. We didn't set a support for iOS, but looking at https://en.wikipedia.org/wiki/IOS_version_history#iOS_10 I think iOS 10 would be good (opinions @yurydelendik?). If range requests work for both those versions, we can remove the compatibility limit altogether I think.

[yurydelendik]

dynamic workaround for issue in another project

If it is possible to introduce a workaround outside main code base (only in compatibility.js), you are welcome to submit a patch. Patching main code base will not be considered.

We only support Safari 8+... I think iOS 10 would be good.

Yes, I agree.

[fooware]

I have done some internal testing and can confirm that removing the Safari limitations as @EugeneSqr stated results in working PDF streaming (with multiple 206 replies) on the following platforms:

iOS 9.3.5
Mozilla/5.0 (iPad; CPU OS 9_3_5 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13G36 Safari/601.1

iOS 10.3.3
Mozilla/5.0 (iPad; CPU OS 10_3_3 like Mac OS X) AppleWebKit/603.3.8 (KHTML, like Gecko) Version/10.0 Mobile/14G60 Safari/602.1

iOS 11.4
Mozilla/5.0 (iPad; CPU OS 11_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.0 Mobile/15E148 Safari/604.1

Safari 11.1.1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/11.1.1 Safari/605.1.15

iOS testing was done on physical devices with a remote web inspector enabled. The benefits was huge for the 30MB PDF I was testing with.

The testing was done with internal code (which this PR stems from) so I am not sure it is fully compatible with your processes, but it's the best I could do.

[Snuffleupagus]

For PDF.js 2.0 we only support Safari 8+.

I cannot remember, and don't have my old notes anymore, if there was ever an explicit decision to support Safari version 8+ specifically. (This is in contrast to IE, where it's decided early on that version 11 would be the lowest supported one.)

According to https://en.wikipedia.org/wiki/Safari_version_history#Safari_8, the last release from the 8 branch was on August 13, 2015. Would it perhaps make sense to actually list version 8, and possibly even 9, as explicitly unsupported (to reduce maintenance burden further)?

[timvandermeij]

@fooware Thank you for looking into this. Given that range requests work again since Safari 9, I think we can remove the compatibility check because Safari 9 is the minimum version we support.

@yurydelendik @Snuffleupagus I have updated https://github.com/mozilla/pdf.js/wiki/Frequently-Asked-Questions#which-browsers-are-supported to reflect that we only support iOS 10 and higher for Safari, as decided above. When I made the overview there, I looked through the commits we did for version 2.0 and found a commit where polyfills were removed that stated that Safari 8+ was now the minimum, but I can't find it quickly anymore. However, since Safari 8 is almost three years old, I think we can safely drop explicity support for it, so I updated the page to reflect this too. I think it's a fair support we have now.

If someone wants to create a patch to remove the compatibility code, please also bump

pdf.js/gulpfile.js

Line 81 in 620da6f

'Safari >= 8',

to >= 9.

[EugeneSqr]

PR #9830 has been created

[Srinithi-23]

@timvandermeij / @yurydelendik We use PDFJS - version - 1.7.331. We wanted to support range requests in safari. We cannot directly use master. So we tried moving to the stable version of PDFJS - 1.9.426 - this version too does not support range requests for safari browser. Is this fix available in any other patch version? Or is there any workaround for us to do this?

[timvandermeij]

Most likely this is only in the pre-release so far: https://github.com/mozilla/pdf.js/releases/tag/2.0.550