[webkit-reviews] review denied: [Bug 75563] remove CSSBorderImageValue : [Attachment 122666] 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 75563: remove CSSBorderImageValue
https://bugs.webkit.org/show_bug.cgi?id=75563

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

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


I think you're missing the file deletes.

> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:508
> +	   RefPtr<CSSValueList> listSlash =
CSSValueList::createSlashSeparated();
> +	   if (imageSlices)
> +	       listSlash->append(imageSlices.release());

The existing test doesn't seem to include test cases for this slash separated
list.  Can you add some test cases before this change to make sure we're not
breaking anything?

Hyatt made it sound like getComputedStyle on webkit-border-image may be
different from getComputedStyle on border-image.

> Source/WebCore/css/CSSParser.cpp:5404
> +	       list->append(m_repeat);
> +	   return list.release();

It looks like this still creates a persistent value for the border-image
instead of being a shorthand (i.e., setting the specific properties instead of
setting border-image).	In the long run, I think we want this to just set the
specific properties, but this is OK for now.