Summary: | Add snap to css3-images image-resolution | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Barr <davidbarr> | ||||||
Component: | CSS | Assignee: | David Barr <davidbarr> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cmarcelo, eric, macpherson, menard, mikelawther, simon.fraser, tony, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
URL: | http://www.w3.org/TR/2012/CR-css3-images-20120417/#the-image-resolution | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 85262, 91467 | ||||||||
Attachments: |
|
Description
David Barr
2012-06-22 00:23:33 PDT
Created attachment 148975 [details]
Patch
Comment on attachment 148975 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=148975&action=review > Source/WebCore/ChangeLog:8 > + No new tests; extended fast/css/image-resolution/image-resolution.html This is OK, but the test covers a lot of different things. If it starts to fail, it'll probably be harder to figure out what exactly is going wrong (800 lines of test!) than if we had a few more focused tests. > LayoutTests/fast/css/image-resolution/image-resolution.html:43 > + else return; Nit: return on next line. > LayoutTests/fast/css/image-resolution/image-resolution.html:48 > - dppx = Math.floor(dppx); > + dppx = Math.floor(dppx + 0.01); Is this due to floating point imprecision? Maybe make a mention of this in the ChangeLog. Created attachment 149216 [details]
Patch for landing
Addressed nit; updated ChangeLog. The single test is getting quite large but I prefer to err on the side of coverage.
Comment on attachment 149216 [details] Patch for landing Clearing flags on attachment: 149216 Committed r121127: <http://trac.webkit.org/changeset/121127> All reviewed patches have been landed. Closing bug. Comment on attachment 149216 [details]
Patch for landing
Why is there no pixel or ref test here? Seems like nothing is testing the actual rendering.
|