Bug 91011

Summary: Simplify css3/flexbox/repaint-rtl-column.html
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: New BugsAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch tony: review+

Description Ojan Vafai 2012-07-11 13:45:35 PDT
Simplify css3/flexbox/repaint-rtl-column.html
Comment 1 Ojan Vafai 2012-07-11 13:46:37 PDT
Created attachment 151762 [details]
Patch
Comment 2 Tony Chang 2012-07-11 13:59:33 PDT
Comment on attachment 151762 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=151762&action=review

> LayoutTests/css3/flexbox/repaint-rtl-column.html:18
> +    height: 350px;

I'm not sure we should hard code a height here.  The original bug was that we would leave some text in the middle of the page that didn't get cleared by the repaint.  You might want to try reverting the code change associated with this test to verify that this new test triggers the failure.

> LayoutTests/css3/flexbox/repaint-rtl-column.html:36
> +    document.getElementById("content").style.webkitFlex = "5";
> +    if (window.testRunner)
> +        testRunner.notifyDone();

The test used to set the flex to 2 different values, but now only sets it once.  Is that the same?
Comment 3 Ojan Vafai 2012-07-11 14:27:24 PDT
Created attachment 151776 [details]
Patch
Comment 4 Ojan Vafai 2012-07-11 14:28:13 PDT
Comment on attachment 151762 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=151762&action=review

>> LayoutTests/css3/flexbox/repaint-rtl-column.html:18
>> +    height: 350px;
> 
> I'm not sure we should hard code a height here.  The original bug was that we would leave some text in the middle of the page that didn't get cleared by the repaint.  You might want to try reverting the code change associated with this test to verify that this new test triggers the failure.

As discussed in person, this is changing the content of the flex item. It's the flex item's height that changes.

>> LayoutTests/css3/flexbox/repaint-rtl-column.html:36
>> +        testRunner.notifyDone();
> 
> The test used to set the flex to 2 different values, but now only sets it once.  Is that the same?

I added back in the two values and the setTimeouts.
Comment 5 Tony Chang 2012-07-11 14:33:17 PDT
Comment on attachment 151776 [details]
Patch

Please mention in the ChangeLog that this also reveals a bug in repainting outline.
Comment 6 Ojan Vafai 2012-07-11 14:52:31 PDT
Committed r122374: <http://trac.webkit.org/changeset/122374>