[webkit-reviews] review denied: [Bug 85451] Add from-image to css3-images image-resolution : [Attachment 147725] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied David Barr <davidbarr at chromium.org>'s
request for review:
Bug 85451: Add from-image to css3-images image-resolution
https://bugs.webkit.org/show_bug.cgi?id=85451

Attachment 147725: Patch
https://bugs.webkit.org/attachment.cgi?id=147725&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=147725&action=review


> Source/WebCore/rendering/style/RenderStyleConstants.h:457
> +enum ImageResolution { ImageResolutionNoFlags = 0, ImageResolutionFromImage
};

I think it would be more clear to have 2 enums. The memory usage is the same
and it will make the values more clear.  E.g., 
enum ImageResolution { ImageResolutionSpecified, ImageResolutionFromImage }
enum ImageResolutionSnap { ImageResolutionNoSnap, ImageResolutionSnapPixels }

> LayoutTests/fast/css/image-resolution/image-resolution.html:70
> -var imgResolutionDppx = 72 / 96;
> -var dimensions = imgWidthPx + 'x' + imgHeightPx + '@' + imgResolutionDppx +
'dppx';
> +var imgResolutionDppx = 0; /* Embedded image resolution data not plumbed
yet. */
> +var dimensions = imgWidthPx + 'x' + imgHeightPx;

Sorry, I just didn't understand why the tests were failing. It's OK to check in
the real test with FAIL in the results as long as it's documented somewhere
with a FIXME.