Fix #284: Handle duplication on NativeFontWatchRunner

NativeFontWatchRunner was too strict: It decides a font being loaded only if document.fonts.load() returns single font. However, there are legit scenarios where multiple fonts of the same CSS name appear. For example, two pieces of JavaScript library happened to load the same font. This change relaxes the criteria to allow more than single fonts.
typekit · Oct 30, 2015 · 8f2b255 · 8f2b255
1 parent b1e6ad9
commit 8f2b255
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 8 deletions.
diff --git a/spec/core/nativefontwatchrunner_spec.js b/spec/core/nativefontwatchrunner_spec.js
@@ -9,6 +9,7 @@ describe('NativeFontWatchRunner', function () {
       inactiveCallback = null,
       nullFont = null,
       sourceSansC = null,
+      sourceSansDup = null,
       elena = null;
 
   beforeEach(function () {
@@ -19,6 +20,7 @@ describe('NativeFontWatchRunner', function () {
 
     nullFont = new Font('__webfontloader_test_3__');
     sourceSansC = new Font('SourceSansC');
+    sourceSansDup = new Font('SourceSansDup');
     elena = new Font('Elena');
   });
 
@@ -40,12 +42,14 @@ describe('NativeFontWatchRunner', function () {
       });
     });
 
-    it('should load font succesfully', function () {
-      var fontWatchRunner = new NativeFontWatchRunner(activeCallback, inactiveCallback,
-          domHelper, sourceSansC),
+    function succesfulLoadingSpec(getFontToBeLoaded, getFontNameToBeLoaded) {
+      var fontToBeLoaded = getFontToBeLoaded(),
+          fontNameToBeLoaded = getFontNameToBeLoaded(),
+          fontWatchRunner = new NativeFontWatchRunner(activeCallback, inactiveCallback,
+          domHelper, fontToBeLoaded),
           ruler = new FontRuler(domHelper, 'abcdef'),
           monospace = new Font('monospace'),
-          sourceSansCFallback = new Font('SourceSansC, monospace'),
+          fallbackFont = new Font(fontNameToBeLoaded + ', monospace'),
           activeWidth = null,
           originalWidth = null,
           finalCheck = false;
@@ -54,7 +58,7 @@ describe('NativeFontWatchRunner', function () {
         ruler.insert();
         ruler.setFont(monospace);
         originalWidth = ruler.getWidth();
-        ruler.setFont(sourceSansCFallback);
+        ruler.setFont(fallbackFont);
         fontWatchRunner.start();
       });
 
@@ -63,7 +67,7 @@ describe('NativeFontWatchRunner', function () {
       });
 
       runs(function () {
-        expect(activeCallback).toHaveBeenCalledWith(sourceSansC);
+        expect(activeCallback).toHaveBeenCalledWith(fontToBeLoaded);
         activeWidth = ruler.getWidth();
         expect(activeWidth).not.toEqual(originalWidth);
 
@@ -80,7 +84,13 @@ describe('NativeFontWatchRunner', function () {
         expect(ruler.getWidth()).not.toEqual(originalWidth);
         expect(ruler.getWidth()).toEqual(activeWidth);
       });
-    });
+    }
+
+    it('should load font succesfully',
+       succesfulLoadingSpec.bind(null, function() { return sourceSansC; }, function() { return 'SourceSansC'; }));
+
+    it('should load font succesfully even if it is duplicated',
+       succesfulLoadingSpec.bind(null, function() { return sourceSansDup; }, function() { return 'SourceSansDup'; }));
 
     it('should attempt to load a non-existing font', function () {
       var fontWatchRunner = new NativeFontWatchRunner(activeCallback, inactiveCallback,

diff --git a/spec/fixtures/fonts/sourcesansdup1.css b/spec/fixtures/fonts/sourcesansdup1.css
diff --git a/spec/fixtures/fonts/sourcesansdup2.css b/spec/fixtures/fonts/sourcesansdup2.css
diff --git a/spec/index.html b/spec/index.html
@@ -11,6 +11,8 @@
     <link rel="stylesheet" href="fixtures/fonts/nullfont3.css">
     <link rel="stylesheet" href="fixtures/fonts/sourcesansa.css">
     <link rel="stylesheet" href="fixtures/fonts/sourcesansc.css">
+    <link rel="stylesheet" href="fixtures/fonts/sourcesansdup1.css">
+    <link rel="stylesheet" href="fixtures/fonts/sourcesansdup2.css">
  </head>
  <body>
     <script>

diff --git a/src/core/nativefontwatchrunner.js b/src/core/nativefontwatchrunner.js
@@ -34,7 +34,11 @@ goog.scope(function () {
         reject(that.font_);
       }, that.timeout_);
     }), doc.fonts.load(this.font_.toCssString(), this.fontTestString_)]).then(function (fonts) {
-      if (fonts.length === 1) {
+      if (fonts.length >= 1) {
+        if (fonts.length !== 1 && console.warn) {
+          console.warn('Font "' + that.font_.toCssString() + '" has duplication.');
+        }
+
         that.activeCallback_(that.font_);
       } else {
         that.inactiveCallback_(that.font_);