[webkit-reviews] review denied: [Bug 136087] [CSS Shapes] Negative raster shape height leads to crash : [Attachment 236842] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 20 08:08:30 PDT 2014

Bem Jones-Bey <bjonesbe at adobe.com> has denied Zoltan Horvath
<zoltan at webkit.org>'s request for review:
Bug 136087: [CSS Shapes] Negative raster shape height leads to crash

Attachment 236842: Patch

------- Additional Comments from Bem Jones-Bey <bjonesbe at adobe.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=236842&action=review

Given the complexity of the test case, I'm not convinced this is the proper

> Source/WebCore/ChangeLog:9
> +	   The original test was provided by the u-szeged fuzzer. I reduced the
> +	   case and handled the negative margin-IntRect case in the shapes

Can you give a better description of what this is fixing?

> +* {

This test seems way too complex, especially since you're matching everything
here. Are you sure you couldn't get something simpler? If you can't reduce it
to simply a single float with shape-outside and a negative height margin box, I
wonder if there is something more complex here than just having the height be
negative. Is there a condition before it even gets to the shapes code that is

> +<button>a</button>

Does this have to be a button for this to fail, or would a <span> work? (Or do
you even need it at all?)

> +<p style="position: absolute; top: 100px;">

I wouldn't muck around with absolute positioning. Just have this paragraph in
here, and dumpAsText() the entire test. You're not gaining anything from it
being a ref test except complexity.

More information about the webkit-reviews mailing list