1193019 - Rename CSSFontFaceLoadEvent to FontFaceSetLoadEvent

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

When the font loading stuff was enabled on release builds in bug 1149381 and merged to beta it broke a bunch of test_interfaces.html tests.  While verifying all the newly exposed globals were correct I noticed that CSSFontFaceLoadEvent is misnamed.  Its now called FontFaceSetLoadEvent in the spec.  This was changed over a year ago:

  https://github.com/w3c/csswg-drafts/commit/48353a72f7bc4bb9cd91102f3a2ce25f7e73b669

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Can we fix this before shipping in release?

Comment 2

[Cameron McCormack (:heycam)]

Thanks for spotting this, Ben.

Comment 3

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b37d53b8f32

Comment 4

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a3b7ffb498e

Comment 5

[Cameron McCormack (:heycam)]

Attachment: patch

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/82b0246cf577

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 8646177 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1028497
[User impact if declined]: script that attempts to use the actual event interface name could break
[Describe test coverage new/current, TreeHerder]: relevant tests updated in this patch
[Risks and why]: low; we are just changing an IDL interface name
[String/UUID change made/needed]: N/A

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/82b0246cf577

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Jorge, this patch has IDL changes and I wanted to get your comment on whether it has a UUID change impact. Is it ok to approve the uplift to Beta on this one? Thanks.

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Changing UUIDs is for .idl (xpidl) files, not .webidl (WebIDL) files.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(That said, it is a Web facing feature change, and I'm not sure what our policy is about uplifting them to beta.)

Comment 12

[Jorge Villalobos [:jorgev] (he/him)]

Yes, as far as add-on compat is concerned, this one isn't an issue.

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to David Baron [:dbaron] (busy Aug. 8-Aug. 30) from comment #11)
> (That said, it is a Web facing feature change, and I'm not sure what our
> policy is about uplifting them to beta.)

We don't have a strict policy. The approach that we should follow is to be informed about the ramifications of these changes. Are we likely to break a lot of web content? Are we likely to break high profile web content? In what way would content break with this change? More generally, what fallout do we expect from this change?

Comment 14

[Cameron McCormack (:heycam)]

It's very unlikely anyone is relying on the old name.  For one thing, the CSS Font Loading API isn't yet available on release (but will be when 41 is released).  For another, this just affects people who are either (a) manually creating an event object using |new CSSFontFaceLoadEvent(...)|, or (b) checking event object instances with |evt instanceof CSSFontFaceLoadEvent|.  I think it's pretty safe to uplift this.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8646177 [details] [diff] [review]
patch

Taking it into beta, this is early in the cycle and we want to release a feature matching the spec.

Comment 16

[Cameron McCormack (:heycam)]

Sherriffs who land this: please update the commit message from r=smaug to r=khuey, thanks. :-)

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/eb10f5d03219

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0aa0af046d24

Comment 19

[Jean-Yves Perrier [:teoli]]

Removing dev-doc-needed. CSSFontFaceLoadEvent didn't reached a release nor was documented as such in MDN (FontFaceLoadEvent is not yet documented)