Bug 66918

Created attachment 105119 [details] patch

Comment on attachment 105119 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=105119&action=review > Source/WebCore/ChangeLog:8 > + No new tests. We should have a test.

Created attachment 105311 [details] added test

Comment on attachment 105311 [details] added test View in context: https://bugs.webkit.org/attachment.cgi?id=105311&action=review > LayoutTests/ChangeLog:8 > + * fast/css/square-button-appearance.html: Added. Test rendering of square-button appearance. You should add this test to test_expectations.txt. Otherwise, layout tests on ports other than chromium-win and mac-snowleopard fail. > LayoutTests/ChangeLog:12 > + * platform/mac-snowleopard/fast/css/square-button-appearance-expected.png: Added. > + * platform/mac-snowleopard/fast/css/square-button-appearance-expected.txt: Added. Do you think these results are not compatible with Lion? If you're not sure about it, I recommend you don't add results for Mac. > LayoutTests/fast/css/square-button-appearance.html:18 > +<h1>Test if square-button appearance is rendered properly.</h1> Please change this line to a comment. Texts prevent test result sharing. <!-- Test if square-button appearance is rendered properly. -->

Created attachment 105320 [details] moved expected files and changed test_expectations

I'm still not sure how this system works. Do I need to add expected results for chromium-mac and chromium-gtk? (In reply to comment #4) > (From update of attachment 105311 [details](apply) [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=105311&action=review > > > LayoutTests/ChangeLog:8 > > + * fast/css/square-button-appearance.html: Added. Test rendering of square-button appearance. > > You should add this test to test_expectations.txt. Otherwise, layout tests on ports other than chromium-win and mac-snowleopard fail. I added to platform/qt/test_expectations.txt and platform/gtk/test_expectations.txt. > > LayoutTests/ChangeLog:12 > > + * platform/mac-snowleopard/fast/css/square-button-appearance-expected.png: Added. > > + * platform/mac-snowleopard/fast/css/square-button-appearance-expected.txt: Added. > > Do you think these results are not compatible with Lion? If you're not sure about it, I recommend you don't add results for Mac. The script, run-webkit-tests, placed it there. It should be the same so I moved it to platform/mac/ > > LayoutTests/fast/css/square-button-appearance.html:18 > > +<h1>Test if square-button appearance is rendered properly.</h1> > > Please change this line to a comment. Texts prevent test result sharing. > <!-- Test if square-button appearance is rendered properly. --> Done.

Comment on attachment 105320 [details] moved expected files and changed test_expectations r- because the patch conflicts and chromium-linux will fail.

Created attachment 105327 [details] added chromium-linux expected file

Do I need to add any more?

Comment on attachment 105327 [details] added chromium-linux expected file forgot to fix conflict

Comment on attachment 105327 [details] added chromium-linux expected file View in context: https://bugs.webkit.org/attachment.cgi?id=105327&action=review > LayoutTests/ChangeLog:10 > + * platform/chromium-linux/fast/css/square-button-appearance-expected.txt: Added. No, we need a PNG result to make all of Chromium bots happy. You don't need to prepare all of expected files. You may just add an entry to test_expectations.txt like other ports.

Created attachment 105328 [details] fixed conflict

Comment on attachment 105328 [details] fixed conflict r- because of a pixel test failure on Chromium-linux.

Created attachment 105332 [details] removed txt, added png

Created attachment 105333 [details] added test to webkit win test_expected.txt

(In reply to comment #15) > Created an attachment (id=105333) [details] > added test to webkit win test_expected.txt It makes no sense. Apple Windows doesn't use new-run-webkit-tests.

Created attachment 105337 [details] revert changes in the last patch

Comment on attachment 105337 [details] revert changes in the last patch ok

Comment on attachment 105337 [details] revert changes in the last patch Rejecting attachment 105337 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ayoutTests/platform/gtk/test_expectations.txt Hunk #1 FAILED at 16. 1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/gtk/test_expectations.txt.rej patching file LayoutTests/platform/qt/test_expectations.txt patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/rendering/RenderThemeChromiumWin.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kent Tamura', u'--force']" exit_code: 1 Full output: http://queues.webkit.org/results/9564013

Created attachment 105452 [details] rebased

Comment on attachment 105452 [details] rebased You may fill out the "Reviewed by" lines in the patch, and may commit it yourself or may enqueue it to commit-queue because the patch already had r+.

(In reply to comment #21) > (From update of attachment 105452 [details](apply) [details]) > You may fill out the "Reviewed by" lines in the patch, and may commit it yourself or may enqueue it to commit-queue because the patch already had r+. Will do so next time.

Comment on attachment 105452 [details] rebased Clearing flags on attachment: 105452 Committed r93953: <http://trac.webkit.org/changeset/93953>

All reviewed patches have been landed. Closing bug.