[webkit-reviews] review denied: [Bug 88047] Remove support for target-densitydpi in the viewport meta tag : [Attachment 145214] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Adam Barth
<abarth at webkit.org>'s request for review:
Bug 88047: Remove support for target-densitydpi in the viewport meta tag
https://bugs.webkit.org/show_bug.cgi?id=88047

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=145214&action=review


r- due to the devicePixelRation = deviceDPI error

>>> Source/WebCore/dom/ViewportArguments.cpp:-371
>>> -	     "Viewport target-densitydpi has to take a number between 70 and
400 as a valid target dpi, try using \"device-dpi\", \"low-dpi\",
\"medium-dpi\" or \"high-dpi\" instead for future compatibility."
>> 
>> I wonder if (at least in the short term) it's worth adding an error message
to note that target-densityDpi isn't supported (in case devs who know we used
to support it are trying to debug)?
> 
> I've got mixed feelings about error messages.  I'm not against it but, want
to just mention that we've had warnings about ; usage in the meta viewport for
a while now.  I'm not sure how many web authors know about or even look at
these errors/warnings.

I think a warning would be fine. It shouldn't hurt anyone.