WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
252712
[IFC][Floats] Floats with clear may be placed incorrectly
https://bugs.webkit.org/show_bug.cgi?id=252712
Summary
[IFC][Floats] Floats with clear may be placed incorrectly
alan
Reported
2023-02-21 17:51:42 PST
ssia
Attachments
Patch
(29.01 KB, patch)
2023-02-21 18:11 PST
,
alan
no flags
Details
Formatted Diff
Diff
[fast-cq]Patch
(33.15 KB, patch)
2023-02-21 21:18 PST
,
alan
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
alan
Comment 1
2023-02-21 18:11:19 PST
Created
attachment 465117
[details]
Patch
alan
Comment 2
2023-02-21 21:18:47 PST
Created
attachment 465118
[details]
[fast-cq]Patch
Antti Koivisto
Comment 3
2023-02-21 21:36:19 PST
Comment on
attachment 465118
[details]
[fast-cq]Patch cool art
EWS
Comment 4
2023-02-22 05:28:16 PST
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]
.
Radar WebKit Bug Importer
Comment 5
2023-02-22 05:29:20 PST
<
rdar://problem/105775276
>
Daniel Holbert
Comment 6
2023-09-26 14:33:52 PDT
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?
Daniel Holbert
Comment 7
2023-09-26 14:51:34 PDT
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!)
alan
Comment 8
2023-09-28 06:52:21 PDT
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.
Daniel Holbert
Comment 9
2023-09-28 10:16:35 PDT
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.
alan
Comment 10
2023-09-28 14:35:58 PDT
> 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.
Daniel Holbert
Comment 11
2023-09-28 15:15:17 PDT
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!
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug