1729089 - New wpt failures in /css/css-font-loading/fontfaceset-has.html (with TypeError `fonts.keys() is not iterable`, due to FontFaceSetIterator not behaving like an Iterable)

Status: RESOLVED DUPLICATE of bug 1311198

(Core :: Layout: Text and Fonts, defect)

Description

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Syncing wpt PR 30322 found new untriaged test failures in CI

Tests Affected

New Tests That Don't Pass

/css/css-font-loading/fontfaceset-has.html
fontfaceset-has: FAIL (Chrome: PASS, Safari: FAIL)

CI Results

Gecko CI (Treeherder)
GitHub PR Head

Notes

These updates will be on mozilla-central once bug 1728932 lands.

Note: this bug is for tracking fixing the issues and is not
owned by the wpt sync bot.

This bug is linked to the relevant tests by an annotation in
https://github.com/web-platform-tests/wpt-metadata. These annotations
can be edited using the wpt interop dashboard
https://jgraham.github.io/wptdash/

If this bug is split into multiple bugs, please also update the
annotations, otherwise we are unable to track which wpt issues are
already triaged. Resolving as duplicate or closing this issue should
be cause the bot to automatically update or remove the annotation.

Comment 1

[Daniel Holbert [:dholbert]]

WPT.fyi link: https://wpt.fyi/results/css/css-font-loading/fontfaceset-has.html

A few observations:
(1) Despite Safari: FAIL in comment 0, it seems Safari is fixing this, per https://github.com/web-platform-tests/wpt/pull/30322#issue-726455679 . (In fact this new test comes from Safari/WebKit folks.)

(2) Safari actually fails differently from us; they fail with an assert_false check failing, whereas we throw a TypeError to say that fonts.keys() is not iterable.

I can reproduce that failure anywhere, e.g. at https://example.org. If I visit that site and type this in my console...

[...document.fonts.keys()]

...I get the same error.

This is sort of odd, because document.fonts.keys() does return an object of type FontFaceSetIterator which sounds extremely iterable. :) (In Chrome, it just returns an object with a slightly-simpler type, Iterator.)

Comment 2

[Daniel Holbert [:dholbert]]

It looks like, for this JS Error's purposes, "iterable" literally means "has a property called Symbol.iterator, which returns a function that can be called", per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/is_not_iterable .

And indeed, this returns Undefined in Firefox (rather than a function):

document.fonts.keys()[Symbol.iterator]

...whereas it returns a callable function in Chrome (and if I invoke that function, it returns an Iterator).

Given that our FontFaceSetIterator object does have a next() function (and hence behaves like an iterator from that standpoint), maybe we just need something akin to the trivial [Symbol.iterator]: function() { return this; } boilerplate on that MDN page, I wonder?

Comment 3

[Daniel Holbert [:dholbert]]

Changing severity to S3 because this seems like a real bug but not especially severe.

Comment 4

[Jonathan Kew [:jfkthame]]

Ideally, we'd probably update FontFaceSet to be declared as setlike<FontFace>, and it would magically work, but I'm not sure how much that actually involves.

Comment 5

[Myles C. Maxfield]

I believe updating to setlike<FontFace> would fix https://bugzilla.mozilla.org/show_bug.cgi?id=1729997 as well.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1729089 - Use setlike<> for FontFaceSet. r=aosmond,jfkthame

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Ok, so I gave a shot at implementing it per spec, and turns out we can't because setlike<> is implemented in terms of a backing object, and that doesn't quite work out, because once we hand out the backing object we can't flush the user fontset anymore (so CSS-connected font-faces aren't updated).

Instead, it seems as of https://github.com/whatwg/webidl/pull/1138, setlike<> it's defined in a different way (which still needs more polish but would allow us to properly implement this).

Edgar / Peter, looking at our existing maplike / setlike usage it seems it should be trivial-ish to update existing consumers to something like that. Do you know if there's a bug tracking implementing those WebIDL spec changes? I could give a shot at doing that in the next couple weeks if nobody can get to it sooner.

Thanks!

Comment 8

[Edgar Chen [:edgar]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Do you know if there's a bug tracking implementing those WebIDL spec changes?

No, I am not aware of any existing bug tracking this change.

Comment 9

[Peter Van der Beken [:peterv]]

(In reply to Jonathan Kew [:jfkthame] from comment #4)

Ideally, we'd probably update FontFaceSet to be declared as setlike<FontFace>, and it would magically work, but I'm not sure how much that actually involves.

See bug 1311198, which contains quite a bit of info on why it's non-trivial to make FontFaceSet a setlike.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

My understanding is that the new spec model would allow us to implement FontFaceSet.has, but I haven't dug into it just yet.

Comment 12

[jeroen.zwartepoorte]

I just ran into this. Works fine in Chromium & WebKit.

Comment 13

[jeroen.zwartepoorte]

Attachment: CleanShot 2024-03-20 at [e-mail]

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Peter Van der Beken [:peterv] from comment #9)

(In reply to Jonathan Kew [:jfkthame] from comment #4)

Ideally, we'd probably update FontFaceSet to be declared as setlike<FontFace>, and it would magically work, but I'm not sure how much that actually involves.

See bug 1311198, which contains quite a bit of info on why it's non-trivial to make FontFaceSet a setlike.

Clearing the NI per the discussion with my team - unfortunately that we didn't have a further quick thought in addition to comment 9. Feel free to request a new NI on me, if you think we should give it a higher priority and take a closer look.

Comment 15

[Daniel Holbert [:dholbert]]

I think we should just consider this a dupe of bug 1311198, which seems to be where the suggested work will happen.