Bug 77001 – border-image should not crash when the source is not specified.

• Component: New Bugs
• Status: RESOLVED FIXED

[Alexis Menard (darktears)]

border-image should not crash when the source is not specified.

[Alexis Menard (darktears)]

Created attachment 123913 [details]
Patch

[Andreas Kling]

Comment on attachment 123913 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123913&action=review

With this change, mapNinePieceImage() becomes very liberal in which argument order it accepts. Are you sure this won't lead to situations where incorrect values slip through the crack?

> Source/WebCore/ChangeLog:10
> +        may happen that you have no value set for it. CSSBorderImage::createBorderImageValue

There is no CSSBorderImage class, the method is simply WebCore::createBorderImageValue.

> Source/WebCore/css/CSSStyleSelector.cpp:4322
> +        if (current->isPrimitiveValue() && !current->isImageValue() && !current->isImageGeneratorValue())

If isPrimitiveValue() is true, isImageGeneratorValue() is going to be false, no need to check the latter.

[Alexis Menard (darktears)]

Created attachment 123946 [details]
Patch

[Alexis Menard (darktears)]

(In reply to comment #2)
> (From update of attachment 123913 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=123913&action=review
> 
> With this change, mapNinePieceImage() becomes very liberal in which argument order it accepts. Are you sure this won't lead to situations where incorrect values slip through the crack?

Well before r105502

http://trac.webkit.org/changeset/105502/trunk/Source/WebCore/css/CSSStyleSelector.cpp

It was already liberal on the order as it was just taking the CSSBorderImageValue object as parameter.

> 
> > Source/WebCore/ChangeLog:10
> > +        may happen that you have no value set for it. CSSBorderImage::createBorderImageValue
> 
> There is no CSSBorderImage class, the method is simply WebCore::createBorderImageValue.

Fixed.

> 
> > Source/WebCore/css/CSSStyleSelector.cpp:4322
> > +        if (current->isPrimitiveValue() && !current->isImageValue() && !current->isImageGeneratorValue())
> 
> If isPrimitiveValue() is true, isImageGeneratorValue() is going to be false, no need to check the latter.

Good catch. Fixed.

[Alexis Menard (darktears)]

Created attachment 123951 [details]
Patch

[Andreas Kling]

Comment on attachment 123951 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=123951&action=review

Looks good, r=me.

> Source/WebCore/css/CSSStyleSelector.cpp:4320
> +        } else if (current->isPrimitiveValue())
> +            // Set the appropriate rules for stretch/round/repeat of the slices.
> +            mapNinePieceImageRepeat(current, image);

Please add { } here, since the comment makes it two lines.

[Alexis Menard (darktears)]

Created attachment 123953 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 123953 [details]
Patch for landing

Clearing flags on attachment: 123953

Committed r105898: <http://trac.webkit.org/changeset/105898>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.