Summary: | REGRESSION: No margin shorthands showing | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Timothy Hatcher <timothy> | ||||||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Yury Semikhatsky <yurys> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | aroben, bdakin, commit-queue, mitz, pfeldman, timothy, yurys | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | All | ||||||||||||||||||
OS: | All | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 28972 | ||||||||||||||||||
Attachments: |
|
Description
Timothy Hatcher
2009-09-04 09:43:51 PDT
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. |