third_party/WebKit/LayoutTests/fast/repaint/table-col-background.html - Issue 1676933004: Table cell background painting

Side by Side Diff: third_party/WebKit/LayoutTests/fast/repaint/table-col-background.html

Issue 1676933004: Table cell background painting (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: LayoutTableCol::clippedOverflowRectForPaintInvalidation fix Created 4 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 <html>	1 <html>

2 <head>	2 <head>

3 <title>Test for http://bugs.webkit.org/show_bug.cgi?id=5821</title>	3 <title>Test for http://bugs.webkit.org/show_bug.cgi?id=5821</title>

4 <script src="resources/text-based-repaint.js" type="text/javascript"></scrip t>	4 <script src="resources/text-based-repaint.js" type="text/javascript"></scrip t>

5 <script type="text/javascript">	5 <script type="text/javascript">

6 function repaintTest()	6 function repaintTest()

7 {	7 {

8 document.getElementById("col").style.backgroundColor = "green";	8 document.getElementById("col").style.backgroundColor = "green";

9 }	9 }

10 </script>	10 </script>

11 </head>	11 </head>

12 <body onload="runRepaintTest()">	12 <body onload="runRepaintTest()">

	13 <div style="height:100px"></div>
	Xianzhu 2016/03/14 20:40:18 Normally we don't change existing tests, but add n Normally we don't change existing tests, but add new tests. However, this change looks OK to me. Will the expectation change? If yes, please do the following to ease review: 1. Schedule a try jobs git cl try and ask for review after the job finishes without unexpected errors. (Normally you might always need this for any CL you asking for review. This is normally better than your local run of all layout tests because: - you can continue to work on other things when the try job is running; - both you and reviewer can see the results; - it will run on all platforms and more environments, not just your own development environment.) 2. Optionally, rebase the expectation before uploading the first patch. I said this is not good for CLs that changes a lot of expectations, but it's good for such CL that might change only a few expectations (especially without pixel changes). atotic1 2016/03/15 16:50:48 I've scheduled a try job, and it failed as expecte Show quoted text On 2016/03/14 20:40:18, Xianzhu wrote: > Normally we don't change existing tests, but add new tests. However, this change > looks OK to me. > > Will the expectation change? If yes, please do the following to ease review: > > 1. Schedule a try jobs > git cl try > and ask for review after the job finishes without unexpected errors. > (Normally you might always need this for any CL you asking for review. > This is normally better than your local run of all layout tests because: > - you can continue to work on other things when the try job is running; > - both you and reviewer can see the results; > - it will run on all platforms and more environments, not just your own > development environment.) > > 2. Optionally, rebase the expectation before uploading the first patch. I said > this is not good for CLs that changes a lot of expectations, but it's good for > such CL that might change only a few expectations (especially without pixel > changes). I've scheduled a try job, and it failed as expected. What's next? Xianzhu 2016/03/15 17:24:39 Please rebaseline expectations and upload again an Show quoted text On 2016/03/15 16:50:48, atotic1 wrote: > On 2016/03/14 20:40:18, Xianzhu wrote: > > Normally we don't change existing tests, but add new tests. However, this > change > > looks OK to me. > > > > Will the expectation change? If yes, please do the following to ease review: > > > > 1. Schedule a try jobs > > git cl try > > and ask for review after the job finishes without unexpected errors. > > (Normally you might always need this for any CL you asking for review. > > This is normally better than your local run of all layout tests because: > > - you can continue to work on other things when the try job is running; > > - both you and reviewer can see the results; > > - it will run on all platforms and more environments, not just your own > > development environment.) > > > > 2. Optionally, rebase the expectation before uploading the first patch. I said > > this is not good for CLs that changes a lot of expectations, but it's good for > > such CL that might change only a few expectations (especially without pixel > > changes). > > I've scheduled a try job, and it failed as expected. What's next? Please rebaseline expectations and upload again and optionally try (which can save time for landing the CL because commit-queue will skip green bots). What I suggested are mainly for ease of review and CL development. For reviewer, it's easier to see expectation diffs from try results. For developer, it's easier to know what tests will fail with the change without running all layout tests on all platforms by himself/herself. For a small number of tests, you can either try first then rebaseline, or upload rebaselined expectation directly. I believe most reviewers would be happen with either way. For the latter way, if there are pixel rebaselines, you'd better tell reviewers what changed. For a large number of layout tests needing rebaselined, it's always better to upload the change without rebaselined expectations first, run try bots, analyze the result, rebaseline tests if the failures are expected [1], upload again and try. Reviewers can see the expectation diffs from the previous try result (normally you need to tell reviewer where to see the results). [1] You have two ways to rebaseline tests 1. run-webkit-tests --reset-results <tests>. Suits: small number of tests; the tests produce the same results on all platforms (or expectation changes on other platforms are know) 2. Mark tests 'NeedsRebaseline' in TestExpectations. Suits: otherwise. A rebaseline bot will rebaseline the tests on all platforms for you after the CL is landed. For this method, you should run try job first and tell reviewer, or tell the reviewer what will change.
13 <table>	14 <table>

14 <col id="col" style="background-color: red;">	15 <col id="col" style="background-color: red;">

15 <tr>	16 <tr>

16 <td style="width: 100px; height: 100px;">	17 <td style="width: 100px; height: 100px;">

17 </td>	18 </td>

18 </tr>	19 </tr>

19 </table>	20 </table>

20 </body>	21 </body>

21 </html>	22 </html>

OLD	NEW

« no previous file with comments | « no previous file | third_party/WebKit/Source/core/layout/LayoutTableCol.cpp » ('j') | third_party/WebKit/Source/core/layout/LayoutTableCol.cpp » ('J')