[webkit-reviews] review denied: [Bug 76697] border-image should be implemented like a shorthand. : [Attachment 123769] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has denied Alexis Menard (darktears)
<alexis.menard at openbossa.org>'s request for review:
Bug 76697: border-image should be implemented like a shorthand.
https://bugs.webkit.org/show_bug.cgi?id=76697

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

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


This will change what element.style.length returns when border-image is set,
right?	Can you add a layout test for that?  That's a good behavior change in
that it will act more like other shorthands.

> Source/WebCore/css/CSSParser.cpp:1483
> +	   if (parseBorderImage(propId, result, important)) {
> +	       if (propId != CSSPropertyBorderImage)
> +		   addProperty(propId, result, important);

I would split the case statements into 2 blocks, one for WebkitBorderImage and
one for BorderImage+WebkitMaskBoxImage.

> Source/WebCore/css/CSSParser.cpp:5304
> +    : m_parser(parser)

Rather than passing in the parser to the constructor, can we just pass in the
parser to commitBorderImage (and forward it on to commitBorderImageProperty)?

> Source/WebCore/css/CSSStyleApplyProperty.cpp:864
> -	       image.setOutset(LengthBox());
> +	       image.setOutset(LengthBox(0));

Was this bug revealed by this change or was this not a problem before?	Maybe
this should be a separate fix with test case?