Bug 80602

Created attachment 131088 [details] patch for this bug positioned-objects pixeltest>reftest conversions done at the hackathon

Created attachment 131096 [details] fix errors in 1st patch file attached. positioned-objects pixeltest>reftest conversions done at the hackathon

Created attachment 131373 [details] re-uploading the patch to re-run gtk EWS re-uploading the 80602 patch to re-run gtk EWS

Comment on attachment 131373 [details] re-uploading the patch to re-run gtk EWS View in context: https://bugs.webkit.org/attachment.cgi?id=131373&action=review > LayoutTests/ChangeLog:8 > + Convert some fast/regions pixel tests to reftests > + https://bugs.webkit.org/show_bug.cgi?id=80602 > + > + Reviewed by NOBODY (OOPS!). Please add a more detailed description what you changed. Also, you changed some test files, these need comments as well. You can put the comments directly next to/under the file names in the ChangeLog. > LayoutTests/fast/regions/positioned-objects-inline-static-in-rtl-regions.html:45 > +is contained in. It should fill the left half of the first region.</p> Should you see red on the page? If so it is a bad tests :) Why did you remove the comment? > LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions.html:-55 > - Some text.<br> > - Some text.<br> > - Some text.<br> > - Some text.<br> > - Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> Some text.<br> </span> Why did you remove the text and the break lines?

Created attachment 132326 [details] revised patch based on the reviewer's feedback

Created attachment 132392 [details] updated comments in the change log updated comments in the change log

Should it be reviewed?

Attachment 132392 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 LayoutTests/ChangeLog:6: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:8: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:9: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:10: Line contains tab character. [whitespace/tab] [5] LayoutTests/ChangeLog:11: Line contains tab character. [whitespace/tab] [5] Total errors found: 5 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 132405 [details] converted tabs to spaces in the ChangeLog

Comment on attachment 132405 [details] converted tabs to spaces in the ChangeLog Attachment 132405 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11966553 New failing tests: fast/regions/positioned-objects-inline-static-spanning-regions.html

Created attachment 132864 [details] fixed the failing test in the previous patch & updated comments in the LayoutTests/ChangeLog

Comment on attachment 132864 [details] fixed the failing test in the previous patch & updated comments in the LayoutTests/ChangeLog Attachment 132864 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12035066 New failing tests: fast/dom/error-to-string-stack-overflow.html fast/regions/positioned-objects-inline-static-spanning-regions.html

Created attachment 132914 [details] updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html a line of text needed to be removed from the LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html file.

Comment on attachment 132914 [details] updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html View in context: https://bugs.webkit.org/attachment.cgi?id=132914&action=review LGTM. r=me > LayoutTests/ChangeLog:12 > + - removed the "no red should be visible..." text from the positioned-objects-inline-static-in-rtl-regions because there was no red in the test file and the statement was irrelevant to the test. > + - removed text from the positioned-objects-inline-static-spanning-regions.html test because it had nothing to do with the test. We based this on the visuals of the test and the fact that the test description made no mention of the text at all Please break lines for change logs in the future.

Comment on attachment 132914 [details] updates to LayoutTests/fast/regions/positioned-objects-inline-static-spanning-regions-expected.html Clearing flags on attachment: 132914 Committed r111545: <http://trac.webkit.org/changeset/111545>

All reviewed patches have been landed. Closing bug.

(In reply to comment #15) > (From update of attachment 132914 [details]) > Clearing flags on attachment: 132914 > > Committed r111545: <http://trac.webkit.org/changeset/111545> This change accidentally removed Mac expected results for two tests that were not converted into reftests. I restored those results in <http://trac.webkit.org/r111989>.

*** Bug 74644 has been marked as a duplicate of this bug. ***