Bug 76931 - RenderSVGShape::strokeContains will fail for some strokes
Summary: RenderSVGShape::strokeContains will fail for some strokes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Stephen Chenney
URL:
Keywords:
Depends on: 71820 77964
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-24 12:36 PST by Stephen Chenney
Modified: 2012-02-07 04:19 PST (History)
5 users (show)

See Also:


Attachments
Patch (14.54 KB, patch)
2012-02-06 14:05 PST, Stephen Chenney
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stephen Chenney 2012-01-24 12:36:32 PST
The method RenderSVGShape::strokeContains has a special case for zero-length-subpaths, and this only handles square end caps. It will give incorrect results for round endcaps on zero-length subpaths.
Comment 1 Dirk Schulze 2012-01-24 14:03:51 PST
Adding Rob who initially worked on zero-length-subpaths and line styles.
Comment 2 Stephen Chenney 2012-02-06 14:05:26 PST
Created attachment 125705 [details]
Patch

A simple fix for this issue and test cases.
Comment 3 Darin Adler 2012-02-06 15:56:06 PST
Comment on attachment 125705 [details]
Patch

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

> LayoutTests/svg/hittest/zero-length-butt-cap-path-expected.txt:1
> +PASSED; PASSED; PASSED; PASSED; PASSED; PASSED; PASSED; PASSED;

Test output is a little cryptic. Tests that make it clear in the output what they are testing are even better.

> LayoutTests/svg/hittest/zero-length-butt-cap-path-expected.txt:6
>  Added: svn:eol-style
>    + LF

We normally use eol-style native instead of LF. Not sure why you’re doing it differently on these files.
Comment 4 Stephen Chenney 2012-02-06 16:10:44 PST
Comment on attachment 125705 [details]
Patch

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

>> LayoutTests/svg/hittest/zero-length-butt-cap-path-expected.txt:1
>> +PASSED; PASSED; PASSED; PASSED; PASSED; PASSED; PASSED; PASSED;
> 
> Test output is a little cryptic. Tests that make it clear in the output what they are testing are even better.

I agree. I was uncertain about the balance of more JS code to construct good test output, vs test compactness. I'll play around with making things clearer, possibly converting all of the svg/hittest tests to use the JS test harnesses that other svg content uses.

>> LayoutTests/svg/hittest/zero-length-butt-cap-path-expected.txt:6
>>    + LF
> 
> We normally use eol-style native instead of LF. Not sure why you’re doing it differently on these files.

Probably just the result of editing in vim on linux. I didn't do anything special at all, or maybe it's a result of copying a problematic existing file as a starting point.
Comment 5 WebKit Review Bot 2012-02-06 17:31:06 PST
Comment on attachment 125705 [details]
Patch

Clearing flags on attachment: 125705

Committed r106882: <http://trac.webkit.org/changeset/106882>
Comment 6 WebKit Review Bot 2012-02-06 17:31:16 PST
All reviewed patches have been landed.  Closing bug.