Bug 264145

Summary: collectLayerFragments() expects a dirty rect relative to the multicol container.
Product: WebKit Reporter: Ahmad Saleem <ahmad.saleem792>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: bfulgham, karlcow, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
rendering in safari, firefox, chrome none

Description Ahmad Saleem 2023-11-03 02:36:27 PDT
Hi Team,

While going through Blink's Commits, I came across another failing test case for Multicolumn content being RTL.

Test Case: https://jsfiddle.net/e15f4a9p/show

Blink Commit: https://chromium.googlesource.com/chromium/blink/+/3abc8fcb44ca7a3a31f37790929808c62aec931f

WebKit Source: https://searchfox.org/wubkat/source/Source/WebCore/rendering/RenderFragmentedFlow.cpp#888

I tried following:

void RenderFragmentedFlow::collectLayerFragments(LayerFragments& layerFragments, const LayoutRect& layerBoundingBox, const LayoutRect& dirtyRectInFragmentedFlow)
{
    ASSERT(!m_fragmentsInvalidated);
    
    LayoutRect enclosingFragmentedFlow(dirtyRectInFragmentedFlow);
    enclosingFragmentedFlow.moveBy(location());
    for (auto& fragment : m_fragmentList)
        fragment.collectLayerFragments(layerFragments, layerBoundingBox, enclosingFragmentedFlow);
}

and

void RenderFragmentedFlow::collectLayerFragments(LayerFragments& layerFragments, const LayoutRect& layerBoundingBox, const LayoutRect& dirtyRect)
{
    ASSERT(!m_fragmentsInvalidated);
    
    LayoutRect dirtyRectInFragmentedFlow(dirtyRect);
    dirtyRectInFragmentedFlow.moveBy(location());
    for (auto& fragment : m_fragmentList)
        fragment.collectLayerFragments(layerFragments, layerBoundingBox, dirtyRectInFragmentedFlow);
}

with corresponding change in .h file as well but it does not make it progress.

____

Just wanted to raise so we can fix it.

Thanks!
Comment 1 Radar WebKit Bug Importer 2023-11-10 01:37:15 PST
<rdar://problem/118226184>
Comment 2 Karl Dubost 2023-11-20 05:34:38 PST
Created attachment 468675 [details]
rendering in safari, firefox, chrome

hmm not sure what are the expectations but

Safari: 
1 2 3 4

Firefox:
4
3
2
1

Chrome:
2 4
1 3

The test text says: 
"Below, the numbers from 1 to 4 should be seen, in ascending order."

which is the case for Safari. 

I have the feeling the test is not fully exhibiting what we would like to know.
Comment 3 Ahmad Saleem 2023-11-20 05:36:29 PST
(In reply to Karl Dubost from comment #2)
> Created attachment 468675 [details]
> rendering in safari, firefox, chrome
> 
> hmm not sure what are the expectations but
> 
> Safari: 
> 1 2 3 4
> 
> Firefox:
> 4
> 3
> 2
> 1
> 
> Chrome:
> 2 4
> 1 3
> 
> The test text says: 
> "Below, the numbers from 1 to 4 should be seen, in ascending order."
> 
> which is the case for Safari. 
> 
> I have the feeling the test is not fully exhibiting what we would like to
> know.

Removed -webkit- prefixes from JSFiddle? Mine still has them so Firefox might have different behavior after removing. Just assuming.
Comment 4 Karl Dubost 2023-11-20 05:41:36 PST
ok the expected test in Blink commit https://chromium.googlesource.com/chromium/blink/+/3abc8fcb44ca7a3a31f37790929808c62aec931f%5E%21/#F0

is showing the number in column.

1
2
3
4
Comment 5 Karl Dubost 2023-11-20 05:47:48 PST
With https://codepen.io/webcompat/pen/YzBegoW

Safari: 
1 2 3 4

Firefox:
1 2 3 4

Chrome:
2 4
1 3