Bug 77001 - border-image should not crash when the source is not specified.
Summary: border-image should not crash when the source is not specified.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexis Menard (darktears)
URL:
Keywords:
Depends on:
Blocks: 76876
  Show dependency treegraph
 
Reported: 2012-01-25 03:24 PST by Alexis Menard (darktears)
Modified: 2012-01-25 10:55 PST (History)
6 users (show)

See Also:


Attachments
Patch (7.17 KB, patch)
2012-01-25 03:33 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (7.06 KB, patch)
2012-01-25 08:42 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch (7.64 KB, patch)
2012-01-25 09:09 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff
Patch for landing (7.59 KB, patch)
2012-01-25 09:14 PST, Alexis Menard (darktears)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2012-01-25 03:24:53 PST
border-image should not crash when the source is not specified.
Comment 1 Alexis Menard (darktears) 2012-01-25 03:33:48 PST
Created attachment 123913 [details]
Patch
Comment 2 Andreas Kling 2012-01-25 08:02:51 PST
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.
Comment 3 Alexis Menard (darktears) 2012-01-25 08:42:09 PST
Created attachment 123946 [details]
Patch
Comment 4 Alexis Menard (darktears) 2012-01-25 08:48:08 PST
(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.
Comment 5 Alexis Menard (darktears) 2012-01-25 09:09:26 PST
Created attachment 123951 [details]
Patch
Comment 6 Andreas Kling 2012-01-25 09:12:29 PST
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.
Comment 7 Alexis Menard (darktears) 2012-01-25 09:14:16 PST
Created attachment 123953 [details]
Patch for landing
Comment 8 WebKit Review Bot 2012-01-25 10:54:56 PST
Comment on attachment 123953 [details]
Patch for landing

Clearing flags on attachment: 123953

Committed r105898: <http://trac.webkit.org/changeset/105898>
Comment 9 WebKit Review Bot 2012-01-25 10:55:01 PST
All reviewed patches have been landed.  Closing bug.