search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.44k stars 656 forks source link

[css-font-loading] Browsers disagree on what it means for a FontFace object to be "CSS-connected", and what effect does it have. #5707

Open emilio opened 3 years ago

emilio commented 3 years ago

Consider a test-case like this:

<!doctype html>
<style>@font-face { font-family: Ahem; src: url(Ahem.ttf); }</style>
<iframe src="about:blank"></iframe>
<script>
onload = function() {
  let css = [...document.fonts][0];
  let nonCss = new FontFace('Ahem2', 'url(Ahem2.ttf)');

  let frameDoc = document.querySelector("iframe").contentDocument;

  frameDoc.fonts.add(nonCss);
  frameDoc.fonts.add(css);
  console.log([...frameDoc.fonts]);
}
</script>

Behavior across browsers differs:

So this is clearly broken. Stuff gets more weird if you do something like document.querySelector("style").remove() and such.

I'd like to do a proposal, and then, separately, discuss what should happen with .add() etc.

The proposal I'd like to add is not to make CSS-connectedness dependent on the current state of the stylesheets of any given document. Basically, making it a per-instance "this came from a CSS rule" flag. This proposal would basically change whether css in the example above would be CSS-connected after doing document.querySelector("style").remove(). This proposal would match WebKit from my understanding of the code.

The reasoning for that is that it's clearly error-prone, and causes synchronous stylesheet updates. Blink gets it wrong because it doesn't look at the right document. Gecko gets the above test-case right per spec, but has other bugs in the area, like incorrectly throwing in this test-case (for the same reason fundamentally, it updates the stylesheets of the wrong document):

<!doctype html>
<style>@font-face { font-family: Ahem; src: url(Ahem.ttf); }</style>
<iframe src="about:blank"></iframe>
<script>
onload = function() {
  let css = [...document.fonts][0];
  let frameDoc = document.querySelector("iframe").contentDocument;
  document.querySelector("style").remove();
  frameDoc.fonts.add(css);
}
</script>

Then there's the discussion of what should happen when you call .add() with a CSS-connected rule. I think we have multiple options:

Thoughts on both proposals, noting that the first is independent of the second?

cc @litherum @tabatkins @lilles @xiaochengh

xiaochengh commented 3 years ago

It looks like replacing "CSS-connected" with "CSS-created".

I'm not sure about the compatibility risk. Maybe we should add a use counter first?

Other than that, I like the proposal, and I'm fine with either Gecko or Blink's behavior regarding .add().

lilles commented 3 years ago

What happens with non-CSS-connected FontFace objects which are added to multiple FontFaceSets? A FontFace has a [[Url]] slot, but if it's added to document.fonts of two different documents with different base urls?

add() says it shouldn't be added twice to the same set, but I couldn't see anything in the spec blocking it from being added to multiple sets.

I see now that there is an issue (Issue 3) in the spec about specifying how base urls work, but it doesn't mention multiple sets/documents. This probably warrants a separate issue.

emilio commented 3 years ago

Yes, that looks like a separate issue. It also crossed my mind because this has been the reason for a lot of discussion for other similar things like Constructable Stylesheets and such.

css-meeting-bot commented 3 years ago

The CSS Working Group just discussed [css-font-loading] Browsers disagree on what it means for a FontFace object to be "CSS-connected", and what effect does it have., and agreed to the following:

The full IRC log of that discussion <dael> Topic: [css-font-loading] Browsers disagree on what it means for a FontFace object to be "CSS-connected", and what effect does it have.
<dael> github: https://github.com/w3c/csswg-drafts/issues/5707
<dael> emilio: Browser behave really oddly. Spec-wise spec says FontFace object once the rule is removed you should be able to use like it's not css-connected. A bit messy, browsers don't impl to the letter. Simplier would be if fontface created for a css rule it is concidered css-created. Then impl and spec are simplier
<dael> emilio: Given behavior is all over the place and it's an edge case it would be nice to simplify
<leaverou> fantasai: 1em evaluated against font-size refers to the *parent* font-size, so there's no conflict there. That's *why* we'd evaluate ems against font-size, to avoid that sort of thing
<dael> TabAtkins: What imacts does ti have on the connection between properties if we just make a flag for css-created
<dael> emilio: afaict is chrome and ff don't allow change of descriptor of fontface object. Per spec it's 2-way mapping. I think FF and Chrome don't impl. I don't see an issue with 2-way mapping as is.
<dael> TabAtkins: If keeping the connection I'm unclear what the change is
<fantasai> leaverou, I think it would be super unexpected if you evaluated em against font-size, unless all of the if clause lengths evaluate against the parent or something
<fantasai> s/font-size/parent font-size/
<dael> emilio: Per spec once you removet he rule from the style sheet, even though om wrapper for rule exists, the fontface object is diconnected from it. Means a lot of functions that need to check for css connection need to also update stylesheets and other expensive stuff
<fantasai> leaverou, and in that case, seems a lot less useful?
<dael> TabAtkins: So if you move a cssom fontface object into another stylesheet in another document so it shows in a different fontface set would that make 1 more object
<dael> emilio: afaict you can't do that. cssom method is strings so you need to stringify
<leaverou> fantasai: then the other options are: a) it evaluates differently per property, so you have partial application b) it evaluates against another property, e.g. width, so you have a cycle in font-size.
<dael> TabAtkins: Okay. If purely when an om rule is created it gets a corresponding object and that's a permanent connection I'm okay with that. Simplication. Fine with me
<Rossen_> q?
<dael> emilio: I think so too
<dael> Rossen_: Other opinions?
<dael> Rossen_: Summary?
<dael> emilio: Proposed: Change css-connected by css created bool where it cannot be unset until removed frmo a document
<fantasai> leaverou, sure I recognize those are bad... but also, it seems to me that the use cases would want to evaluate against the element itself
<dael> Rossen_: Objecitons?
<dael> RESOLVED: Change css-connected by css created bool where it cannot be unset until removed from a document
<dael> emilio: Another issue in this. That was changing definition. Now what happens to document.fonts.add with that object
<fantasai> leaverou, if that's not the case and evaluating the parent font size is useful and expected, great, but if not, then making it implementable isn't actually solving the problem
<dael> emilio: It's in the same issue, but needs different resolution
<dael> emilio: it's when it's from a rule created in another document. I think blink does nothing. Spec says throw which is what gecko does. Doesn't match WK or blink. Happy to use either, both are reasonable.
<dael> emilio: It does nothing if called on same document which is odd. I think easiest is follow blink
<dael> TabAtkins: Meaning it doesn't get added to set?
<dael> emilio: Right
<dael> TabAtkins: No opinion on throw or ignore. Whichever
<dael> emilio: I don't care either. Throw to do nothing is a bit easier for use. Doing nothing to throwing may break. No strong opinion. Whatever gets faster interop
<dael> TabAtkins: Seems rare to do this. I suspect we could move to throw and I would prefer because it's an error. Do we have an issue to fix in chrome?
<dael> emilio: I'm okay change to throw
<dael> TabAtkins: I'll try that and talk to Rune. If not we'll come back
<dael> Rossen_: With my TAG hat I'd argue strongly for throwing. There's a pretty clear guidance on this pattern. Should do most observable. Let's not have silent error. I agree with prop
<dael> Rossen_: Objections?
<dael> RESOLVED: Have it throw an error
<dael> RESOLVED: have document.fonts.add when called with css create fontface object throw an error
<leaverou> fantasai: If you look at the use cases wrt WC, none of them really seems to need ems. We just need to define what happens when someone uses it that way. I agree that parent is not that useful, but not sure the alternatives are better. I'm hoping there might be a 4th alternative we haven't considered, but I think the top priority would be to make sure that either the entire @if is applied or none of it, even if some values become less useful in
<leaverou> conditions.
tabatkins commented 3 years ago

I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1159471 to track the issue mentioned in the minutes where Chrome silently ignores attempts to add a CSS-connected FontFace to other documents' FontFaceSets.