Bug 91011 - Simplify css3/flexbox/repaint-rtl-column.html
Summary: Simplify css3/flexbox/repaint-rtl-column.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-11 13:45 PDT by Ojan Vafai
Modified: 2012-07-11 14:52 PDT (History)
1 user (show)

See Also:

Attachments
Patch (35.71 KB, patch)
2012-07-11 13:46 PDT, Ojan Vafai 		no flags Details | Formatted Diff | Diff
Patch (35.86 KB, patch)
2012-07-11 14:27 PDT, Ojan Vafai 		tony: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>