Bug 254183

Summary: Let siblings layout if an adjacent float may no longer affect them
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: Layout and RenderingAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, changseok, esprehn+autocc, ews-watchlist, glenn, koivisto, kondapallykalyan, pdr, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
[fast-cq]Patch none

Description Ahmad Saleem 2023-03-20 14:15:10 PDT 
Hi Team,

While going through Blink's commit, I came across another potential merge and bug fix.

Blink Commit - https://chromium.googlesource.com/chromium/src.git/+/789b5e528fb3bd9ca44341de46547c45355652ab

Test Case - https://jsfiddle.net/kqj93hp5/show

^ Failing in Safari Technology Preview 165, passes after adding following in WebKit Source:

        fb = std::max(fb, childBlockFlow->lowestFloatLogicalBottom());

In WebKit Source after this as L969:

https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderBlockFlow.cpp#968

Just wanted to raise so we can fix it.

Thanks!
Comment 1 Ahmad Saleem 2023-03-20 16:25:50 PDT 
(In reply to Ahmad Saleem from comment #0)
> Hi Team,
> 
> While going through Blink's commit, I came across another potential merge
> and bug fix.
> 
> Blink Commit -
> https://chromium.googlesource.com/chromium/src.git/+/
> 789b5e528fb3bd9ca44341de46547c45355652ab
> 
> Test Case - https://jsfiddle.net/kqj93hp5/show
> 
> ^ Failing in Safari Technology Preview 165, passes after adding following in
> WebKit Source:
> 
>         fb = std::max(fb, childBlockFlow->lowestFloatLogicalBottom());
> 
> In WebKit Source after this as L969:
> 
> https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderBlockFlow.
> cpp#968
> 
> Just wanted to raise so we can fix it.
> 
> Thanks!

Merging this as it is, led to crashes in my local build (It happened when I try to looked into old bugs with 'negative margin' reference to see, if they are now fixed with this or not.

Additionally, then I took the patch reference to search whether there is any follow-up on Chromium / Blink and noticed that it had regression in performance test and got superseded by this:

https://chromium.googlesource.com/chromium/src.git/+/4a961b8eca27af4108358a30753197754316524b
Comment 2 Radar WebKit Bug Importer 2023-03-27 14:16:17 PDT 
<rdar://problem/107283741>
Comment 3 zalan 2023-04-01 07:08:54 PDT 
Created attachment 465729 [details]
[fast-cq]Patch
Comment 4 zalan 2023-04-01 07:10:32 PDT 
Different approach as I am not convinced that the blink patch is correct.
Comment 5 EWS 2023-04-01 10:04:02 PDT 
Committed 262481@main (5673db1ac5ba): <https://commits.webkit.org/262481@main>

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