Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
384042:
Rebaselinine fast/overflow/004.html as it seems to have new expected text/im... (Closed)
|
Created:
11 years, 1 month ago by kinuko (google) Modified:
9 years, 7 months ago CC:
chromium-reviews_googlegroups.com, darin (slow to review), pam+watch_chromium.org Base URL:
http://src.chromium.org/svn/trunk/src/ Visibility:
Public. |
DescriptionRebaselinine fast/overflow/004.html as the expected results have been updated upstream (bugs.webkit.org/show_bug.cgi?id=31455).
BUG=10432
TEST=none
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=33176
Patch Set 1 #Patch Set 2 : '' #
Messages
Total messages: 16 (0 generated)
Reviewers: gwilson, jon, Pam, Description: Rebaselinine fast/overflow/004.html as it apparently has new expected text/image that match our results. (The bug 10277 has already been closed as WONTFIX though.) BUG=10277 TEST=none Please review this at http://codereview.chromium.org/384042 SVN Base: http://src.chromium.org/svn/trunk/src/ Affected files: M webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum M webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.png M webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum M webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.png M webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt M webkit/tools/layout_tests/test_expectations.txt Index: webkit/tools/layout_tests/test_expectations.txt =================================================================== --- webkit/tools/layout_tests/test_expectations.txt (revision 31645) +++ webkit/tools/layout_tests/test_expectations.txt (working copy) @@ -926,7 +926,6 @@ BUG20507 : LayoutTests/http/tests/uri/resolve-encoding-relative.html = FAIL -BUG10432 WIN LINUX : LayoutTests/fast/overflow/004.html = FAIL //also BUG24863 BUG9071 MAC WIN SLOW : LayoutTests/editing/selection/drag-to-contenteditable-iframe.html = FAIL PASS Index: webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.png =================================================================== Cannot display: file marked as a binary type. svn:mime-type = image/png Index: webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum =================================================================== --- webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum (revision 31645) +++ webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum (working copy) @@ -1 +1 @@ -21453c209e1f38e0404d8bb37daf6a53 \ No newline at end of file +d96d1d76b9d140ed8dab47b1515dbfb0 \ No newline at end of file Index: webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt =================================================================== --- webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt (revision 31645) +++ webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt (working copy) @@ -1,12 +1,12 @@ -layer at (0,0) size 800x600 - RenderView at (0,0) size 800x600 -layer at (0,0) size 800x538 - RenderBlock {HTML} at (0,0) size 800x538 - RenderBody {BODY} at (8,8) size 784x522 - RenderBlock (anonymous) at (0,0) size 784x20 +layer at (0,0) size 785x1040 + RenderView at (0,0) size 785x600 +layer at (0,0) size 785x1040 + RenderBlock {HTML} at (0,0) size 785x1040 + RenderBody {BODY} at (8,8) size 769x1024 + RenderBlock (anonymous) at (0,0) size 769x20 RenderText {#text} at (0,0) size 669x19 text run at (0,0) width 669: "The two green blocks below should be identical and should each take up half the width of the browser window." - RenderBlock (floating) {DIV} at (390,20) size 394x502 [bgcolor=#008000] [border: (1px solid #000000)] + RenderBlock (floating) {DIV} at (383,20) size 386x502 [bgcolor=#008000] [border: (1px solid #000000)] RenderText {#text} at (0,0) size 0x0 -layer at (8,28) size 394x502 clip at (9,29) size 392x500 - RenderBlock {DIV} at (0,20) size 394x502 [bgcolor=#008000] [border: (1px solid #000000)] +layer at (8,530) size 386x502 clip at (9,531) size 384x500 + RenderBlock {DIV} at (0,522) size 386x502 [bgcolor=#008000] [border: (1px solid #000000)] Index: webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.png =================================================================== Cannot display: file marked as a binary type. svn:mime-type = image/png Index: webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum =================================================================== --- webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum (revision 31645) +++ webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum (working copy) @@ -1 +1 @@ -3bcf7ffcb662757ed912ecdb23e1f859 \ No newline at end of file +6f8aea58c89b89d2ed63dc50340c89c2 \ No newline at end of file
I'm likely not the best reviewer for this, and Jon no longer works on the project. Adding Dimitri, who may be a better resource than I. On 2009/11/11 14:16:40, Kinuko Yasuda wrote: > Reviewers: gwilson, jon, Pam, > > Description: > Rebaselinine fast/overflow/004.html as it apparently has new expected > text/image > that match our results. (The bug 10277 has already been closed as WONTFIX > though.) > > BUG=10277 > TEST=none > > > Please review this at http://codereview.chromium.org/384042 > > SVN Base: http://src.chromium.org/svn/trunk/src/ > > Affected files: > M > webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > M > webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.png > M > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > M > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.png > M > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > M webkit/tools/layout_tests/test_expectations.txt > > > Index: webkit/tools/layout_tests/test_expectations.txt > =================================================================== > --- webkit/tools/layout_tests/test_expectations.txt (revision 31645) > +++ webkit/tools/layout_tests/test_expectations.txt (working copy) > @@ -926,7 +926,6 @@ > > BUG20507 : LayoutTests/http/tests/uri/resolve-encoding-relative.html = FAIL > > -BUG10432 WIN LINUX : LayoutTests/fast/overflow/004.html = FAIL > > //also BUG24863 > BUG9071 MAC WIN SLOW : > LayoutTests/editing/selection/drag-to-contenteditable-iframe.html = FAIL > PASS > Index: > webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.png > =================================================================== > Cannot display: file marked as a binary type. > svn:mime-type = image/png > Index: > webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > =================================================================== > --- > webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > > (revision 31645) > +++ > webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > > (working copy) > @@ -1 +1 @@ > -21453c209e1f38e0404d8bb37daf6a53 > \ No newline at end of file > +d96d1d76b9d140ed8dab47b1515dbfb0 > \ No newline at end of file > Index: > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > =================================================================== > --- > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > > (revision 31645) > +++ > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > > (working copy) > @@ -1,12 +1,12 @@ > -layer at (0,0) size 800x600 > - RenderView at (0,0) size 800x600 > -layer at (0,0) size 800x538 > - RenderBlock {HTML} at (0,0) size 800x538 > - RenderBody {BODY} at (8,8) size 784x522 > - RenderBlock (anonymous) at (0,0) size 784x20 > +layer at (0,0) size 785x1040 > + RenderView at (0,0) size 785x600 > +layer at (0,0) size 785x1040 > + RenderBlock {HTML} at (0,0) size 785x1040 > + RenderBody {BODY} at (8,8) size 769x1024 > + RenderBlock (anonymous) at (0,0) size 769x20 > RenderText {#text} at (0,0) size 669x19 > text run at (0,0) width 669: "The two green blocks below should > be identical and should each take up half the width of the browser window." > - RenderBlock (floating) {DIV} at (390,20) size 394x502 > [bgcolor=#008000] [border: (1px solid #000000)] > + RenderBlock (floating) {DIV} at (383,20) size 386x502 > [bgcolor=#008000] [border: (1px solid #000000)] > RenderText {#text} at (0,0) size 0x0 > -layer at (8,28) size 394x502 clip at (9,29) size 392x500 > - RenderBlock {DIV} at (0,20) size 394x502 [bgcolor=#008000] [border: (1px > solid #000000)] > +layer at (8,530) size 386x502 clip at (9,531) size 384x500 > + RenderBlock {DIV} at (0,522) size 386x502 [bgcolor=#008000] [border: > (1px solid #000000)] > Index: > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.png > =================================================================== > Cannot display: file marked as a binary type. > svn:mime-type = image/png > Index: > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > =================================================================== > --- > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > > (revision 31645) > +++ > webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > > (working copy) > @@ -1 +1 @@ > -3bcf7ffcb662757ed912ecdb23e1f859 > \ No newline at end of file > +6f8aea58c89b89d2ed63dc50340c89c2 > \ No newline at end of file > >
I'm likely not the best reviewer for this, and Jon no longer works on the project. Adding Dimitri, who may be a better resource than I. On 2009/11/11 14:16:40, Kinuko Yasuda wrote: > Reviewers: gwilson, jon, Pam, > Description: > Rebaselinine fast/overflow/004.html as it apparently has new expected > text/image > that match our results. (The bug 10277 has already been closed as WONTFIX > though.) > BUG=10277 > TEST=none > Please review this at http://codereview.chromium.org/384042 > SVN Base: http://src.chromium.org/svn/trunk/src/ > Affected files: > M webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > M webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.png > M webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > M webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.png > M webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > M webkit/tools/layout_tests/test_expectations.txt > Index: webkit/tools/layout_tests/test_expectations.txt > =================================================================== > --- webkit/tools/layout_tests/test_expectations.txt (revision 31645) > +++ webkit/tools/layout_tests/test_expectations.txt (working copy) > @@ -926,7 +926,6 @@ > BUG20507 : LayoutTests/http/tests/uri/resolve-encoding-relative.html = > FAIL > -BUG10432 WIN LINUX : LayoutTests/fast/overflow/004.html = FAIL > //also BUG24863 > BUG9071 MAC WIN SLOW : > LayoutTests/editing/selection/drag-to-contenteditable-iframe.html = FAIL > PASS > Index: webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.png > =================================================================== > Cannot display: file marked as a binary type. > svn:mime-type = image/png > Index: webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > =================================================================== > --- webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > (revision 31645) > +++ webkit/data/layout_tests/platform/chromium-linux/LayoutTests/fast/overflow/004-expected.checksum > (working copy) > @@ -1 +1 @@ > -21453c209e1f38e0404d8bb37daf6a53 > \ No newline at end of file > +d96d1d76b9d140ed8dab47b1515dbfb0 > \ No newline at end of file > Index: webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > =================================================================== > --- webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > (revision 31645) > +++ webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.txt > (working copy) > @@ -1,12 +1,12 @@ > -layer at (0,0) size 800x600 > - RenderView at (0,0) size 800x600 > -layer at (0,0) size 800x538 > - RenderBlock {HTML} at (0,0) size 800x538 > - RenderBody {BODY} at (8,8) size 784x522 > - RenderBlock (anonymous) at (0,0) size 784x20 > +layer at (0,0) size 785x1040 > + RenderView at (0,0) size 785x600 > +layer at (0,0) size 785x1040 > + RenderBlock {HTML} at (0,0) size 785x1040 > + RenderBody {BODY} at (8,8) size 769x1024 > + RenderBlock (anonymous) at (0,0) size 769x20 > RenderText {#text} at (0,0) size 669x19 > text run at (0,0) width 669: "The two green blocks below > should > be identical and should each take up half the width of the browser > window." > - RenderBlock (floating) {DIV} at (390,20) size 394x502 > [bgcolor=#008000] [border: (1px solid #000000)] > + RenderBlock (floating) {DIV} at (383,20) size 386x502 > [bgcolor=#008000] [border: (1px solid #000000)] > RenderText {#text} at (0,0) size 0x0 > -layer at (8,28) size 394x502 clip at (9,29) size 392x500 > - RenderBlock {DIV} at (0,20) size 394x502 [bgcolor=#008000] [border: > (1px > solid #000000)] > +layer at (8,530) size 386x502 clip at (9,531) size 384x500 > + RenderBlock {DIV} at (0,522) size 386x502 [bgcolor=#008000] [border: > (1px solid #000000)] > Index: webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.png > =================================================================== > Cannot display: file marked as a binary type. > svn:mime-type = image/png > Index: webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > =================================================================== > --- webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > (revision 31645) > +++ webkit/data/layout_tests/platform/chromium-win/LayoutTests/fast/overflow/004-expected.checksum > (working copy) > @@ -1 +1 @@ > -3bcf7ffcb662757ed912ecdb23e1f859 > \ No newline at end of file > +6f8aea58c89b89d2ed63dc50340c89c2 > \ No newline at end of file http://codereview.chromium.org/384042
I don't understand. Looking at the description of thest, the new baseline is clearly wrong. Why are we doing this?
I don't understand. Looking at the description of thest, the new baseline is clearly wrong. Why are we doing this? http://codereview.chromium.org/384042
It's true that the description of the test doesn't match the results, but seems like WebKit also behaves like ours now: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac/fast/overflow/0... http://trac.webkit.org/changeset/47255#file25 The code change clearly puts the second div under the first div if its width doesn't fit in the left space. I wasn't able to find out how the flow-div and overflow-div are positioned specifically in the spec, but it's also how FF and IE works. If it's not the expected result we should fix RenderBlock upstream to take the overflow into consideration when it calculates the clear delta. Otherwise probably we should fix the comment in the test. Which way should we take? On Thu, Nov 12, 2009 at 2:35 AM, <dglazkov@chromium.org> wrote: > I don't understand. Looking at the description of thest, the new baseline > is > clearly wrong. Why are we doing this? > > > http://codereview.chromium.org/384042 >
Ah. That makes sense. We should rebaseline as you suggested, then.
On 2009/11/12 04:01:21, Dimitri Glazkov wrote: > Ah. That makes sense. We should rebaseline as you suggested, then. Can you give me a LGTM if it looks good? Thanks,
LGTM.
As much as I'd like to see the test changed so its result matches its description, LGTM too, since that's out of our purview. - Pam
kinuko: Maybe file a bug to change the test upstream? :DG< On Thu, Nov 12, 2009 at 8:03 PM, <pam@chromium.org> wrote: > As much as I'd like to see the test changed so its result matches its > description, LGTM too, since that's out of our purview. > > - Pam > > http://codereview.chromium.org/384042 >
Sure, sounds good, I'm going to file a bug upstream. On Fri, Nov 13, 2009 at 1:07 PM, Dimitri Glazkov <dglazkov@chromium.org>wrote: > kinuko: Maybe file a bug to change the test upstream? > > :DG< > > On Thu, Nov 12, 2009 at 8:03 PM, <pam@chromium.org> wrote: > > As much as I'd like to see the test changed so its result matches its > > description, LGTM too, since that's out of our purview. > > > > - Pam > > > > http://codereview.chromium.org/384042 > > >
Filed a bug upstream, revised the test to make it match the description (rather than updating the description), and updated the patch in chromium (patchset 2). Could you take a look at them again? Related upstream bug: https://bugs.webkit.org/show_bug.cgi?id=31580 Thanks, On 2009/11/13 05:25:47, kinuko wrote: > Sure, sounds good, I'm going to file a bug upstream. > > On Fri, Nov 13, 2009 at 1:07 PM, Dimitri Glazkov <dglazkov@chromium.org>wrote: > > > kinuko: Maybe file a bug to change the test upstream? > > > > :DG< > > > > On Thu, Nov 12, 2009 at 8:03 PM, <mailto:pam@chromium.org> wrote: > > > As much as I'd like to see the test changed so its result matches its > > > description, LGTM too, since that's out of our purview. > > > > > > - Pam > > > > > > http://codereview.chromium.org/384042 > > > > > >
LGTM. Thanks for staying on this :)
Submitted in r33176. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
