Bug 140581

Summary: setting the textContent of an element in a flex-box can trigger improper clipping of redraws
Product: WebKit Reporter: Gordon Sheridan <gordon_sheridan>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED CONFIGURATION CHANGED    
Severity: Normal CC: ahmad.saleem792, gordon_sheridan, hyatt, simon.fraser, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: Unspecified   
Attachments:
Description Flags
simple test case
none
Test reduction. none

Description Gordon Sheridan 2015-01-16 18:01:59 PST
Created attachment 244826 [details]
simple test case

In the attached test case, if the textContent of the "description" element is changed without affecting its line wrapping, as the images are removed, the images are not erased or drawn properly.

The portion that is improperly clipped, seems to be related to the padding of the "inner-div" element.
Comment 1 zalan 2015-01-16 19:18:52 PST
Created attachment 244832 [details]
Test reduction.
Comment 2 zalan 2015-01-16 21:22:56 PST
RenderBlockFlow::layoutInlineChildren() returns with the correct repaint rect (repaintLogicalTop, repaintLogicalBottom) as the result of deleting line boxes, but later in RenderBlockFlow::layoutBlock() we intersect this repaint rect with the newly computed size when clipping is present.
// Don't allow this rect to spill out of our overflow box.
repaintRect.intersect(LayoutRect(LayoutPoint(), size()));
So while shrinking the flow, we fail to clean up the bits that's outside of the current rect (unless we delete from the bottom/end of the line as then the repaint issued by the box-to-be-deleted takes care of the "outside" part.)
I wonder if we've got a different codepath responsible to clean up.
Comment 3 zalan 2015-01-19 16:42:05 PST
(In reply to comment #2)
> RenderBlockFlow::layoutInlineChildren() returns with the correct repaint
> rect (repaintLogicalTop, repaintLogicalBottom) as the result of deleting
> line boxes, but later in RenderBlockFlow::layoutBlock() we intersect this
> repaint rect with the newly computed size when clipping is present.
> // Don't allow this rect to spill out of our overflow box.
> repaintRect.intersect(LayoutRect(LayoutPoint(), size()));
> So while shrinking the flow, we fail to clean up the bits that's outside of
> the current rect (unless we delete from the bottom/end of the line as then
> the repaint issued by the box-to-be-deleted takes care of the "outside"
> part.)
> I wonder if we've got a different codepath responsible to clean up.
It's a bit more complicated than that. This is specific to flexboxes.
1. in RenderFlexibleBox::layoutBlock() we call layoutFlexItems() to layout children.
2. We hit the absolute positioned (with top:0 bottom:0) RenderBlockFlow where the <img> renderers are.
3. RenderBlockFlow::layoutBlock() calls layoutInlineChildren() which comes back with proper top and bottom repaint coords (they correctly cover the change inside the flow)
4. Since this absolute positioned RenderBlockFlow() also has overflow clip, we intersect the repaint rect with the computed size().  
5 We compute the height of the flow using the parent's height (stretch to 100% with top:0 bottom:0). However, at this point the parent flexbox's height is not final yet. Intersecting the repaint rect with the updated size() results in ignoring the bottom dirty area.
6. RenderFlexibleBox calls updateLogicalHeight() and repositionLogicalHeightDependentFlexItems()
7. Another round for RenderBlockFlow::layoutBlock().
8. layoutInlineChildren() now returns with 0, 0 (top, bottom) dirty rect. (no change since the last pass)
9. Flow's height gets update properly, but since the top and bottom dirty rect == 0, we end up not repaint anything during this second pass.
Comment 4 Ahmad Saleem 2023-07-05 12:03:14 PDT
I am not able to reproduce this bug anymore and using ‘Test Reduction’ after repaint, I only get one broken image icon on top of ‘bar’, which is expected result and it is also the case in Safari 16.5.1 and WebKit ToT. Chrome Canary 117 and Firefox Nightly 117 are also same as Safari.

@Alan - I am marking this as ‘RESOLVED CONFIGURATION CHANGED”, please reopen if it is reproducible for you.
Comment 5 zalan 2023-07-05 15:13:02 PDT
IFC progression.