Fix: Safari font rendering issue (#332)

Should fix mozilla/pdf.js#8700 and https://cloud.box.com/s/xd92oda655j808n0zvklga0qt2tltn6v on Safari
box · Aug 25, 2017 · 847cd8c · 847cd8c
1 parent bebdcec
commit 847cd8c
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 33 deletions.
diff --git a/src/lib/Browser.js b/src/lib/Browser.js
@@ -61,7 +61,7 @@ class Browser {
      * @public
      * @param {string} type - The mime type to check.
      * @param {string} probability - Should either be 'maybe' or 'probably'
-     * @return {boolean} true if browser supports a particular type
+     * @return {boolean} True if browser supports a particular type
      */
     static canPlayType(type, probability) {
         let elem;
@@ -84,7 +84,7 @@ class Browser {
      *
      * @public
      * @param {string} mime - information about the AVC profile codec
-     * @return {boolean} true if browser supports HTML5 H264 main video playback
+     * @return {boolean} True if browser supports HTML5 H264 main video playback
      */
     static canPlayH264(mime) {
         return Browser.canPlayType(mime, 'maybe') || Browser.canPlayType(mime, 'probably');
@@ -97,7 +97,7 @@ class Browser {
      * Also see {@link https://developer.mozilla.org/en-US/docs/HTML/Supported_media_formats|MDN}
      *
      * @public
-     * @return {boolean} true if browser supports HTML5 H264 baseline video playback
+     * @return {boolean} True if browser supports HTML5 H264 baseline video playback
      */
     static canPlayH264Baseline() {
         return Browser.canPlayH264(MIME_H264_BASELINE);
@@ -110,7 +110,7 @@ class Browser {
      * Also see {@link https://developer.mozilla.org/en-US/docs/HTML/Supported_media_formats|MDN}
      *
      * @public
-     * @return {boolean} true if browser supports HTML5 H264 main video playback
+     * @return {boolean} True if browser supports HTML5 H264 main video playback
      */
     static canPlayH264Main() {
         return Browser.canPlayH264(MIME_H264_MAIN);
@@ -123,7 +123,7 @@ class Browser {
      * Also see {@link https://developer.mozilla.org/en-US/docs/HTML/Supported_media_formats|MDN}
      *
      * @public
-     * @return {boolean} true if browser supports HTML5 H264 high video playback
+     * @return {boolean} True if browser supports HTML5 H264 high video playback
      */
     static canPlayH264High() {
         return Browser.canPlayH264(MIME_H264_HIGH);
@@ -137,7 +137,7 @@ class Browser {
      * Also see {@link https://developer.mozilla.org/en-US/docs/HTML/Supported_media_formats|MDN}
      *
      * @public
-     * @return {boolean} true if browser supports HTML5 MP3 audio playback
+     * @return {boolean} True if browser supports HTML5 MP3 audio playback
      */
     static canPlayMP3() {
         return Browser.canPlayType('audio/mpeg', 'maybe') || Browser.canPlayType('audio/mpeg', 'probably');
@@ -149,7 +149,7 @@ class Browser {
      * to the H264 container (since we use H264 and not webm)
      *
      * @public
-     * @return {boolean} true if dash is usable
+     * @return {boolean} True if dash is usable
      */
     static canPlayDash() {
         const mse = global.MediaSource;
@@ -168,7 +168,7 @@ class Browser {
      * Checks the browser for Media Source Extensions support
      *
      * @public
-     * @return {boolean} true if MediaSource extensions are enabled
+     * @return {boolean} True if MediaSource extensions are enabled
      */
     static hasMSE() {
         return !!global.MediaSource;
@@ -178,7 +178,7 @@ class Browser {
      * Returns true if the browser supports webgl or experimental webgl
      *
      * @public
-     * @return {boolean} - returns true if the browser supports WebGL
+     * @return {boolean} True if the browser supports WebGL
      */
     static hasWebGL() {
         if (!gl) {
@@ -226,7 +226,7 @@ class Browser {
      * the Box3DRuntime for displaying Model Preview
      *
      * @public
-     * @return {boolean} true if browser fully supports Model Previewing
+     * @return {boolean} True if browser fully supports Model Previewing
      */
     static supportsModel3D() {
         if (!Browser.hasWebGL()) {
@@ -242,7 +242,7 @@ class Browser {
      * Determines if flash is installed.
      *
      * @public
-     * @return {boolean} true if browser has flash
+     * @return {boolean} True if browser has flash
      */
     static hasFlash() {
         let hasFlash = false;
@@ -258,7 +258,7 @@ class Browser {
      * Returns true if the browser supports SVG
      *
      * @public
-     * @return {boolean} is svg supported
+     * @return {boolean} True if svg supported
      */
     static hasSVG() {
         return document.implementation.hasFeature('http://www.w3.org/TR/SVG11/feature#BasicStructure', '1.1');
@@ -279,7 +279,7 @@ class Browser {
      * Returns whether the browser is a mobile browser.
      *
      * @public
-     * @return {boolean} true if browser supports download
+     * @return {boolean} True if browser supports download
      */
     static isMobile() {
         // Relying on the user agent to avoid desktop browsers on machines with touch screens.
@@ -292,7 +292,7 @@ class Browser {
      * Currently not supported by IE11 or Safari 10.0 http://caniuse.com/#feat=download
      *
      * @public
-     * @return {boolean} true if browser supports download
+     * @return {boolean} True if browser supports download
      */
     static canDownload() {
         return !Browser.isMobile() || (!window.externalHost && 'download' in document.createElement('a'));
@@ -302,7 +302,7 @@ class Browser {
      * Returns whether or not the device is running IOS
      *
      * @public
-     * @return {boolean} true if the device is running IOS
+     * @return {boolean} True if the device is running IOS
      */
     static isIOS() {
         return /(iPad|iPhone|iPod)/g.test(userAgent);
@@ -312,20 +312,33 @@ class Browser {
      * Returns whether or not the device is running Android
      *
      * @public
-     * @return {boolean} true if the device is running Android
+     * @return {boolean} True if the device is running Android
      */
     static isAndroid() {
         return /Android/g.test(userAgent);
     }
 
     /**
-     * Returns whether or not the device is running IOS 10.3.x that has Font Ligature rendering issue.
+     * Returns whether or not the device is a laptop/desktop Mac
      *
      * @public
-     * @return {boolean} Whether device is running 10.3.x
+     * @return {boolean} Whether device is a Mac
      */
-    static isIOSWithFontIssue() {
-        return Browser.isIOS() && /(?:OS\s)10_3/i.test(userAgent);
+    static isMac() {
+        return /Macintosh; Intel Mac OS X/g.test(userAgent);
+    }
+
+    /**
+     * Returns whether or not the device is running IOS 10.3.x or browser is desktop Safari, both of which have Font
+     * Ligature rendering issues due to the font loading API.
+     *
+     * @public
+     * @return {boolean} Whether device or browser have font ligature issues
+     */
+    static hasFontIssue() {
+        return (
+            (Browser.isIOS() && /(?:OS\s)10_3/i.test(userAgent)) || (Browser.isMac() && Browser.getName() === 'Safari')
+        );
     }
 }
 

diff --git a/src/lib/__tests__/Browser-test.js b/src/lib/__tests__/Browser-test.js
@@ -461,22 +461,42 @@ describe('lib/Browser', () => {
         });
     });
 
-    describe('isIOSWithFontIssue()', () => {
+    describe('isMac()', () => {
+        it('should return true if device is a Mac', () => {
+            Browser.overrideUserAgent('(Macintosh; Intel Mac OS X 10_10_4)');
+            const mac = Browser.isMac();
+            expect(mac).to.be.true;
+        });
+
+        it('should return false if device is not a Mac', () => {
+            Browser.overrideUserAgent('(Windows NT 6.1; Win64; x64; rv:47.0)');
+            const mac = Browser.isMac();
+            expect(mac).to.be.false;
+        });
+    });
+
+    describe('hasFontIssue()', () => {
         it('should return true if device is on ios and is OS 10.3.XX', () => {
             Browser.overrideUserAgent('iPhone OS 10_3_90 safari/2');
-            const hasIssue = Browser.isIOSWithFontIssue();
+            const hasIssue = Browser.hasFontIssue();
             expect(hasIssue).to.be.true;
         });
 
         it('should return false if device is on ios and is not OS 10.3.XX', () => {
             Browser.overrideUserAgent('iPhone OS 10_5_90 safari/2');
-            const hasIssue = Browser.isIOSWithFontIssue();
+            const hasIssue = Browser.hasFontIssue();
             expect(hasIssue).to.be.false;
         });
 
-        it('should return false if device is on ios and is not mobile', () => {
-            Browser.overrideUserAgent('DesktopDevice OS 10_3_90 safari/18902374701347589235');
-            const hasIssue = Browser.isAndroid();
+        it('should return true if device is a Mac running Safari', () => {
+            Browser.overrideUserAgent('Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) AppleWebKit/600.7.12 (KHTML, like Gecko) Version/8.0.7 Safari/600.7.12');
+            const hasIssue = Browser.hasFontIssue();
+            expect(hasIssue).to.be.true;
+        });
+
+        it('should return false if device is a Mac and not on Safari', () => {
+            Browser.overrideUserAgent('Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2311.90 Safari/537.36');
+            const hasIssue = Browser.hasFontIssue();
             expect(hasIssue).to.be.false;
         });
     });

diff --git a/src/lib/viewers/doc/DocBaseViewer.js b/src/lib/viewers/doc/DocBaseViewer.js
@@ -549,7 +549,7 @@ class DocBaseViewer extends BaseViewer {
 
         // Disable font faces on IOS 10.3.X
         // @NOTE(JustinHoldstock) 2017-04-11: Check to remove this after next IOS release after 10.3.1
-        PDFJS.disableFontFace = PDFJS.disableFontFace || Browser.isIOSWithFontIssue();
+        PDFJS.disableFontFace = PDFJS.disableFontFace || Browser.hasFontIssue();
 
         // Disable range requests for files smaller than MINIMUM_RANGE_REQUEST_FILE_SIZE (25MB) for
         // previews outside of the US since the additional latency overhead per range request can be

diff --git a/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js b/src/lib/viewers/doc/__tests__/DocBaseViewer-test.js
@@ -842,14 +842,14 @@ describe('src/lib/viewers/doc/DocBaseViewer', () => {
             expect(PDFJS.externalLinkRel).to.equal('noopener noreferrer nofollow');
         });
 
-        // @NOTE(JustinHoldstock) 2017-04-11: Check to remove this after next IOS release after 10.3.1
-        it('should test user agent if on Safari Mobile for IOS 10.3', () => {
-            const getStub = sandbox.stub(Browser, 'isIOSWithFontIssue').returns(true);
+        // @NOTE(JustinHoldstock) 2017-04-11: Check to remove or modify this after next IOS release after 10.3.1 or
+        // Safari version
+        it('should test if browser has font rendering issue', () => {
+            PDFJS.disableFontFace = false;
+            sandbox.mock(Browser).expects('hasFontIssue').returns(true);
+
             docBase.setupPdfjs();
 
-            // Mobile stub cannot be called if get stub is never called.
-            // See note for this test, for more info.
-            expect(getStub).to.be.called;
             expect(PDFJS.disableFontFace).to.be.true;
         });