Bug 156035 – [Font loading] document.fonts.check() always return true

• Component: New Bugs
• Status: REOPENED

[Arthur]

Some times document.fonts.check() doesn't work. More specifically, it works, but always returns -- true. I do not know what is the reason, because it works fine in our staging env and fails on my local and production envs.

Repro:

1. Go to https://m.oumy.com/#/
2. Browser should load 3 Roboto fonts. None is loading.
3. Run in console: document.fonts.check('16px Roboto') and document.fonts.check('16px Zzz') -- both returns -- true.

4. Go to https://html5.oumy.com:8080/#/
5. See that 3 Roboto fonts are loaded.
6. Run in console: document.fonts.check('16px Roboto') and document.fonts.check('16px Zzz') -- both returns -- true.

[Arthur]

Just for more context:

"Zzz" is unknowns font, i.e. random test to test
"Roboto" is font which I actually wont to use.

By some reason, when page is loading `document.fonts.check()` returns different values for those 2 different sites.

On page load, on both sites, `document.fonts.check()` returns true for both fonts ("Zzz", "Roboto"). On a second site where Roboto is loaded it's understandable why `fonts.check()` returns true (because font was really loaded), in all other cases everything is weird :-)

My code of loading fonts is approximately like this:

if (!document.fonts.check(...)) { // e.g. "16px Roboto"
  document.fonts.load(...).then((face) => document.fonts.add(face));
}

[Arthur]

Oh, https://html5.oumy.com:8080/ actually has an old version of site running and it's not using Fonts Loading API to load fonts, so fonts loading works. Now it looks like `document.fonts.check()` always just returns true.

Sorry for misleading.

[Jon Lee]

(In reply to comment #2)
> Oh, https://html5.oumy.com:8080/ actually has an old version of site running
> and it's not using Fonts Loading API to load fonts, so fonts loading works.

Did you mean to say m.oumy.com is not using the Fonts Loading API?

[Arthur]

No. m.oumy.com is production and it uses Fonts Loading API and it works everywhere (where supported)except on Safari Tech Preview.

https://html5.oumy.com:8080/ is a staging env and it seems to have an older version of site, because I was testing everything locally. Staging uses fallback (fonts loading library) to load fonts, this is why it works in Tech Preview.

[Myles C. Maxfield]

I've investigated https://m.oumy.com/#/.

The page creates the following @font-face blocks:
@font-face {
    font-family: AppFont;
    font-style: normal;
    font-weight: 400;
    src: local('Roboto'), local('Roboto-Regular'), local('sans-serif')
}

@font-face {
    font-family: AppFont;
    font-style: normal;
    font-weight: 500;
    src: local('Roboto Medium'), local('Roboto-Medium'), local('sans-serif-medium')
}

@font-face {
    font-family: AppFont;
    font-style: normal;
    font-weight: 700;
    src: local('Roboto Bold'), local('Roboto-Bold'), local('sans-serif-bold')
}

@font-face {
    font-family: AppFont;
    font-style: normal;
    font-weight: 300;
    src: local('Roboto Light'), local('Roboto-Light'), local('sans-serif-light')
}

It then calls document.fonts.check() three times, with the following arguments:
"normal 400 48px Roboto"
"normal 500 48px Roboto"
"normal 700 48px Roboto"


Because the above @font-face blocks are not using the font-family "Roboto", the check() call is not interested in them.

Therefore, no fonts are found that match the query (because OS X doesn't have any preinstalled fonts named "Roboto". In this situation, the spec [1] says: "If font face list is empty, or all fonts in the font face list either have a status attribute of "loaded" or are system fonts, return true."

We are correctly returning "true" here.

[1] https://drafts.csswg.org/css-font-loading/#font-face-set-check

[Myles C. Maxfield]

https://html5.oumy.com:8080/#/ also includes the additional @font-faces:

@font-face {
  font-family: 'Roboto';
  font-style: normal;
  font-weight: 400;
  src: local('Roboto'), local('Roboto-Regular'), url(https://fonts.gstatic.com/s/roboto/v15/CrYjSnGjrRCn0pd9VQsnFOvvDin1pK8aKteLpeZ5c0A.woff) format('woff');
}
@font-face {
  font-family: 'Roboto';
  font-style: normal;
  font-weight: 500;
  src: local('Roboto Medium'), local('Roboto-Medium'), url(https://fonts.gstatic.com/s/roboto/v15/RxZJdnzeo3R5zSexge8UUbO3LdcAZYWl9Si6vvxL-qU.woff) format('woff');
}
@font-face {
  font-family: 'Roboto';
  font-style: normal;
  font-weight: 700;
  src: local('Roboto Bold'), local('Roboto-Bold'), url(https://fonts.gstatic.com/s/roboto/v15/d-6IYplOFocCacKzxwXSOLO3LdcAZYWl9Si6vvxL-qU.woff) format('woff');
}


This explains why the fonts are loaded successfully on that page.

[Myles C. Maxfield]

I'm going to mark this bug as already working correctly. If you think there's more to do, please feel free to re-open.

[Arthur]

First all, it's working in every other browser and isn't working in Safari Tech Preview, so obviously, something is wrong with it.

Second, you are wrong.



> "If font face list is empty, or all fonts in the font face list either have a status attribute of "loaded" or are system fonts, return true."

Returned font face list can be empty only if such font was found and then was removed from matched list because it didn't match unicode-range test. Otherwise "found flag" is set to false and check() must return false

So if Roboto isn't system font and it's not already loaded in FontFaceSet (document.fonts), implementation here must return false.

I really hate that I have to explain spec for you.

Next, this is font faces used by both m.oumy.com and html5.oumy.com: https://gist.github.com/NekR/fb3db9bd88f605f1e106a9069ae95cfc

This is default font defined for the app, it's fallback and as you see, it's not loading any fonts.

Roboto fonts are loaded from script with either Font Loading API (if available) or with fallback library. As I said, html5.oumy.com has older version of app and therefore has different code. I just reviewed that old code again, and thing is, it's actually using Fonts Loading API, but not using document.fonts.check(). This is why fonts are loaded on html5.oumy.com (not using document.fonts.check()) and not loading on m.oumy.com (using document.fonts.check()).

Furthermore, AppFont isn't related here at all. Once fonts are loaded (or script determines that browser already has those fonts), 'fonts' class is added to document body, which overrides AppFont usage and tells to page to use only Roboto.


One more thing -- go to any page in Safari Tech Preview and run document.fonts.check('16px sdfjsdkfjskdj') and it will return true. I doubt that it's correct behavior.



And the last, please treat this comment as the last comment on this tracker because I do not like those "WontFixes" just because you were not able to figure out what is going on. Why I should care about your browser if you do not care?

I really do not need it as a developer, I always can say to my client "this is browser bug and I cannot do anything here", but instead, I am trying to help.

[Ryosuke Niwa]

(In reply to comment #8)
>
> One more thing -- go to any page in Safari Tech Preview and run
> document.fonts.check('16px sdfjsdkfjskdj') and it will return true. I doubt
> that it's correct behavior.

This behavior matches that of Firefox (44.0.2 and 48.0a1) as well as the spec text as far as I can tell.

I agree this is somewhat counterintuitive. Perhaps we might want to raise this concern in the CSS WG. This could be an interoperability issue already given Chrome returns false in this case if content authors are assuming that's the right behavior.

[Timothy Hatcher]

Marking as "WORKSFORME" instead of "WONTFIX" or "FIXED" since that is the right resolution here (unless we determine otherwise after talking to the spec authors.)

[Myles C. Maxfield]

Reopening due to spec issues.

[Tab Atkins]

(In reply to comment #5)
> Therefore, no fonts are found that match the query (because OS X doesn't
> have any preinstalled fonts named "Roboto". In this situation, the spec [1]
> says: "If font face list is empty, or all fonts in the font face list either
> have a status attribute of "loaded" or are system fonts, return true."
> 
> We are correctly returning "true" here.
> 
> [1] https://drafts.csswg.org/css-font-loading/#font-face-set-check

Nope, read step *3* of the algorithm, which checks the "found faces" flag.  It lets check() distinguish between "found some faces, but threw them out, no valid matches" and "you clearly typo'd, there's no font by that name anywhere on this page".

The check() method in the example in your comment should *throw*, not return true.  (Sorry I haven't specified the error that should be thrown; feel free to suggest what it should be.)

[Myles C. Maxfield]

I'd like to provide some background here.

1. Before May 19, 2004 the spec stated that this situation should return true.
2. On May 19, 2004, the spec switched to stating that this situation should return false. This Git revision [1] switched it.
3. On May 27, 2005, the spec switch to stating that this situation should thrown an exception, but it didn't state which kind of exception should be thrown. This Git revision [2] switched it. This decision was made based on the W3C thread starting at [3], where Cameron and John from Mozilla state their issues with this situation returning false. Tab from Google argues that this situation should be treated as an error condition, and he modified the spec to state that an exception should be thrown.

The latest development is item #3, so the current state of the spec [4] states that this situation should throw an exception, but doesn't state which kind of exception should be thrown. The TR version of the spec [5] is stale, and was created between items #2 and #3, so it states that this situation should return false.

WebKit generally implements the latest version of any spec, so the most relevant document is the current draft [4]. That being said, currently no browsers implement the behavior stated in the spec. WebKit generally favors interoperability over exactly following specs. IE / Edge don't implement this API, Firefox implements the spec as of item #1, and Chrome implements the spec as of item #2. The Safari Technical Preview currently agrees with Firefox.

This was not an accident. I agree with the arguments that Cameron and John made, and I am participating in the discussion about this issue in the W3C. You can see my response to the thread at [6]. I argue that the behavior of item #1 should be restored.

Because this issue is still in flux (and is clearly contentious, given the amount of changes this issue in the spec has undergone), I will participate in discussions at the W3C on this topic until they conclude. At that point, I'll update WebKit (and this bug).

[1] https://github.com/w3c/csswg-drafts/commit/35f2b975e8c914ffa901fbfba7f8ca93d18bfdba
[2] https://github.com/w3c/csswg-drafts/commit/e73686097b5699745e58c42788be17b1821aa38f
[3] https://lists.w3.org/Archives/Public/www-style/2015May/0002.html
[4] https://drafts.csswg.org/css-font-loading/
[5] https://www.w3.org/TR/css-font-loading/
[6] http://lists.w3.org/Archives/Public/www-style/2016Apr/0000.html

[Myles C. Maxfield]

(In reply to comment #13)
> I'd like to provide some background here.
> 
> 1. Before May 19, 2004 the spec stated that this situation should return
> true.
> 2. On May 19, 2004, the spec switched to stating that this situation should
> return false. This Git revision [1] switched it.
> 3. On May 27, 2005, the spec switch to stating that this situation should
> thrown an exception, but it didn't state which kind of exception should be
> thrown. This Git revision [2] switched it. This decision was made based on
> the W3C thread starting at [3], where Cameron and John from Mozilla state
> their issues with this situation returning false. Tab from Google argues
> that this situation should be treated as an error condition, and he modified
> the spec to state that an exception should be thrown.
> 
> The latest development is item #3, so the current state of the spec [4]
> states that this situation should throw an exception, but doesn't state
> which kind of exception should be thrown. The TR version of the spec [5] is
> stale, and was created between items #2 and #3, so it states that this
> situation should return false.
> 
> WebKit generally implements the latest version of any spec, so the most
> relevant document is the current draft [4]. That being said, currently no
> browsers implement the behavior stated in the spec. WebKit generally favors
> interoperability over exactly following specs. IE / Edge don't implement
> this API, Firefox implements the spec as of item #1, and Chrome implements
> the spec as of item #2. The Safari Technical Preview currently agrees with
> Firefox.
> 
> This was not an accident. I agree with the arguments that Cameron and John
> made, and I am participating in the discussion about this issue in the W3C.
> You can see my response to the thread at [6]. I argue that the behavior of
> item #1 should be restored.
> 
> Because this issue is still in flux (and is clearly contentious, given the
> amount of changes this issue in the spec has undergone), I will participate
> in discussions at the W3C on this topic until they conclude. At that point,
> I'll update WebKit (and this bug).
> 
> [1]
> https://github.com/w3c/csswg-drafts/commit/
> 35f2b975e8c914ffa901fbfba7f8ca93d18bfdba
> [2]
> https://github.com/w3c/csswg-drafts/commit/
> e73686097b5699745e58c42788be17b1821aa38f
> [3] https://lists.w3.org/Archives/Public/www-style/2015May/0002.html
> [4] https://drafts.csswg.org/css-font-loading/
> [5] https://www.w3.org/TR/css-font-loading/
> [6] http://lists.w3.org/Archives/Public/www-style/2016Apr/0000.html

Whoops, s/2004/2014/g & s/2005/2015/g

[Adrian Gawor]

Today I've been struck by this issue, so I did a a bit of digging and to make long story short I've stumbled upon this line of code https://github.com/WebKit/webkit/blob/950143da027e80924b4bb86defa8a3f21fd3fb1e/Source/WebCore/css/CSSFontFaceSet.cpp#L406
Correct me if I'm wrong but in my personal and not so humble opinion it's plain wrong as check method should produce false as well in case of:
   face.get().status() == CSSFontFace::Status::Loading

[Adrian Gawor]

On top of above Firefox impl (https://hg.mozilla.org/mozilla-central/file/default/layout/style/FontFaceSet.cpp#l358) and as well Chromium (https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/third_party/blink/renderer/core/css/font_face_set.cc#221 and https://chromium.googlesource.com/chromium/src.git/+/refs/heads/master/third_party/blink/renderer/core/css/css_segmented_font_face.cc#180) pretty much boils down to status != Loaded 
Would be great to have unified approach to that (read as same as other browsers). Currently in WebKit FontFaceSet::check returns true even if font face is still loading.

[Sam Sneddon [:gsnedders]]

*** Bug 184532 has been marked as a duplicate of this bug. ***

[Sam Sneddon [:gsnedders]]

rdar://39380289

[Vitor Roriz]

Shouldn't this be closed since https://github.com/w3c/csswg-drafts/issues/5744 decided for not throwing anymore for this case?

[Ahmad Saleem]

(In reply to Vitor Roriz from comment #19)
> Shouldn't this be closed since
> https://github.com/w3c/csswg-drafts/issues/5744 decided for not throwing
> anymore for this case?

This links to following Chromium / Blink commit: https://chromium-review.googlesource.com/c/chromium/src/+/4913179 ; which refers following WPT: https://wpt.fyi/results/css/css-fonts/fallback-url-to-local.html?label=experimental&label=master&aligned=

Which STP184 fails.