Bug 89745 – Add snap to css3-images image-resolution

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[David Barr]

The css3-images module is at candidate recommendation.
http://www.w3.org/TR/2012/CR-css3-images-20120417/#the-image-resolution

I propose to introduce image-resolution, initially behind a compile-time flag.

Advertised on webkit-dev:
http://thread.gmane.org/gmane.os.opendarwin.webkit.devel/20505

As a fifth step, add support for the snap value.

[David Barr]

Created attachment 148975 [details]
Patch

[Tony Chang]

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.

[David Barr]

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.

[WebKit Review Bot]

Comment on attachment 149216 [details]
Patch for landing

Clearing flags on attachment: 149216

Committed r121127: <http://trac.webkit.org/changeset/121127>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Fraser (smfr)]

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.

[David Barr]

Created bug 91467 to address Simon's comment.