No margin shorthand is showing, when they should. Steps: 1) Visit webkit.org 2) Inspect #icon 3) Look at the styles. Results: No margin shorthand showing. Expected: A margin shorthand of 0px.
Created attachment 39068 [details] Bug
Created attachment 39069 [details] Expected
Created attachment 39265 [details] test page This regression was introduced with the fix for https://bugs.webkit.org/show_bug.cgi?id=28635. I've attached a page where where margins shorthand is empty. If 'margin: 0px;' is moved to the top of the #icon style declaration block the margin shorthand becomes 0px in the Web Inspector.
Created attachment 39266 [details] patch
Comment on attachment 39266 [details] patch We can write a LayoutTest for this, no?
Created attachment 39273 [details] patch with layout test
Comment on attachment 39273 [details] patch with layout test You need to fill in the info in the LayoutTests ChangeLog.
Created attachment 39274 [details] patch with layout test Added LayoutTest/ChangeLog entry.
(In reply to comment #7) > (From update of attachment 39273 [details]) > You need to fill in the info in the LayoutTests ChangeLog. Done.
Comment on attachment 39274 [details] patch with layout test Tests are better when they contain the expected results in them. In this case, I don't know if your test results are correct or not, and new implementors woudl have to compare to the existing -expected.txt files to know if their implementations are failing or passing this. javascript-only tests are also preferred in cases like this: http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree Numerous examples of js-only tests in resources/ directories containing a TEMPLATE.html file. This is OK, but the test definitely could be slicker. :)
Created attachment 39328 [details] patch with pure js layout test
(In reply to comment #10) > (From update of attachment 39274 [details]) > Tests are better when they contain the expected results in them. In this case, > I don't know if your test results are correct or not, and new implementors > woudl have to compare to the existing -expected.txt files to know if their > implementations are failing or passing this. > > javascript-only tests are also preferred in cases like this: > http://trac.webkit.org/wiki/Writing%20Layout%20Tests%20for%20DumpRenderTree > > Numerous examples of js-only tests in resources/ directories containing a > TEMPLATE.html file. > > This is OK, but the test definitely could be slicker. :) Thanks for your suggestion. I'm new to the layout tests and it's not obvious which of them can be considered as reference implementation. I refactored the test so that it uses TEMPLATE.html.
Comment on attachment 39328 [details] patch with pure js layout test Rejecting patch 39328 from commit-queue. ('WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1') Running build-dumprendertree Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests Testing 11263 test cases. (48)Address already in use: make_sock: could not bind to address 127.0.0.1:8080 no listening sockets available, shutting down Unable to open logs Timed out waiting for httpd to start at WebKitTools/Scripts/run-webkit-tests line 1466, <IN> line 20152.
Comment on attachment 39328 [details] patch with pure js layout test Sorry, that's clearly not your fault. :) I happen to be running another service on 8080 on the commit machine when your patch went through the queue.
Comment on attachment 39328 [details] patch with pure js layout test Clearing flags on attachment: 39328 Committed r48436: <http://trac.webkit.org/changeset/48436>
All reviewed patches have been landed. Closing bug.