Bug 252712

Created attachment 465117 [details]
Patch

Created attachment 465118 [details]
[fast-cq]Patch

Comment on attachment 465118 [details]
[fast-cq]Patch

cool art

Committed 260674@main (af963178be93): <https://commits.webkit.org/260674@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465118 [details].

<rdar://problem/105775276>

Hi zalan -- we Mozilla folks were looking at testcases-that-WebKit-has-added-recently-that-Firefox-fails, and we came across this bug's testcase/reference-case (in the patch that landed here).

But when looking closer, we noticed that Safari and Chrome render these test files the same way that we do -- everyone is in agreement on how the testcase and reference case render, and everyone is in agreement that the testcase/reference-case render differently from each other.

Here are the test files in question:
https://github.com/WebKit/WebKit/blob/af963178be93b40684f69cf4753845cdcceb2fdd/LayoutTests/fast/inline/inline-content-with-float-clear.html
https://github.com/WebKit/WebKit/blob/af963178be93b40684f69cf4753845cdcceb2fdd/LayoutTests/fast/inline/inline-content-with-float-clear-expected.html

Here are live versions of those files, for convenience:
https://bugzilla.mozilla.org/attachment.cgi?id=9355181
https://bugzilla.mozilla.org/attachment.cgi?id=9355182

Do you know what's going on? Maybe we're misunderstanding something, or maybe these tests only pass under some non-default WebKit configuration?

Aha, sorry -- I was testing Safari 16.6 when seeing the test fail (that's the latest WebKit I have locally).

It seems the testcase does indeed render the same as the reference case in Safari 17 and in WebKit's Tech Preview, based on feedback from my colleagues and based on BrowserStack testing just now.

So, that mostly answers my question, and things aren't as confusing as I had thought they were. :)

While I'm posting here, though, I might as well suggest moving this testcase into WPT, e.g. to https://wpt.fyi/results/css/CSS2/floats , if you think it's a situation where the new behavior is spec-compliant and should be interoperable?  Because it seems we had interop on the prior way that this testcase was rendering, and we no longer do. (I haven't dug in far enough to have an opinion about which behavior is preferable, but the ASCII art in https://github.com/WebKit/WebKit/commit/af963178be93b40684f69cf4753845cdcceb2fdd does look helpful and gives me an idea about the before/after situation here -- thanks for that!)

Hi Daniel,
Thank you! I can't remember off the top of my head what triggered this change, whether it was some seemingly broken layout or just matching our legacy line layout behavior but yeah I indeed should have moved this test over to WPT. Will look into it. Thanks again.

Thanks!

I am a bit curious what prompted the change, since it was a step *away* from interoperability (since Safari 16.6-and-earlier rendered your testcase the same way as Firefox and Chrome), and it's introducing a situation where `clear:left` shifts a float *upwards* on the page instead of downwards, which is a bit counterintuitive (though I do understand the reasoning).

It looks like CSS2 does technically allow either behavior, as I discussed in the second half of https://bugzilla.mozilla.org/show_bug.cgi?id=1855303#c4 -- but the spec does have a note saying that there's an intent to eventually tighten that requirement, in favor of one or the other behavior.  So probably at this point, this test as a hypothetical WPT would want to have `.tentative` in the name.

If there's a strong reason to favor the behavior you've switched to here, it'd be great if you could file an issue in https://github.com/w3c/csswg-drafts/ to explain the reasons you prefer the new behavior.  And if there's agreement, and there's no substantial webcompat fallout, we can get it specced in css-floats-3 and implemented in other browsers.

> If there's a strong reason to favor the behavior you've switched to here,
> it'd be great if you could file an issue in
there was no strong reasoning AFAIR. I think it did fall out of a refactoring change where I completely decoupled _inline_ and _float_ layouts which in this case resulted the float avoider clearing _floats_ and not _line_boxes_ anymore. I don't have a strong preference. will file.

Gotcha, OK.

If you do end up reverting back to the prior interoperable behavior (it sounds like you might?), then it'd be especially-great to morph the testcase into a WPT test (whether .tentative or not) with an amended reference-case, since it's apparently exercising float behavior that was interoperable and hence maybe-required-to-render-the-web, but is lacking in test-coverage. (if we had WPT tests, you would have discovered them by seeing test failures when landing this)

I appreciate the prompt responses - thanks!