351798 - Path mouseover fires outside svg

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Richard Smith]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; ONS Internet Explorer 6.1; .NET CLR 1.1.4322)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1b2) Gecko/20060821 Firefox/2.0b2

I have a multi-part SVG consisting of a number of sections each one defined within its own svg. The "map" section contains geographic area polygons defined via path statements. Sometimes the coordinates of the polygons exceed the viewbox dimensions, i.e. only part of the polygon is inside the map area. These are clipped to the inner svg but the mouseover event still fires in the area outside the SVG where the polygon would have extended were it not clipped. Sounds complex but very easy to see with the attached testcase.

Reproducible: Always

Steps to Reproduce:
1. Load the attached testcase into Firefox
2. Hold the mouse over the text "HELLO".
3.

Actual Results:  
The mouseover associated with the path inside the square fires ("polygon mouseover" alert)

Expected Results:  
The mouseover associated with the text element ("hello" alert) should have fired.

This problem occurs with Firefox 2.2 but not with Opera 9.1. I have worked around it by modifying the path coordinates in XSL so that they do not lie outside the viewbox.

Comment 1

[Richard Smith]

Attachment: svg testcase

Comment 2

[tor]

Attachment: use clip rectangle in hit detection

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can't you share code for the GetFrameForPoint too?

Comment 4

[tor]

Attachment: image hit testing testcase

Comment 5

[tor]

Attachment: image testcase with included image data

Comment 6

[tor]

Attachment: fix up image hit testing, makes code sharing less likely

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 238668 [details] [diff] [review]
fix up image hit testing, makes code sharing less likely

+  PRBool rv = PR_TRUE;

Call this "result" or something else, since we normally use rv for nsresults.

Comment 8

[tor]

Attachment: checkin version with variable name change

Comment 9

[tor]

Checked in.

Comment 10

[tor]

Attachment: backported branch patch

Comment 11

[Robert Longson [:longsonr]]

Comment on attachment 242220 [details] [diff] [review]
backported branch patch

I don't know whether GDI+ still compiles/runs on the branch. It won't after this with the cairo code directly in nsSVGUtils.cpp. I assume you don't regard this as a problem.

>+already_AddRefed<nsIDOMSVGMatrix>
>+nsSVGImageFrame::GetImageTransform()
>+{

>+  // default to the defaults
>+  if (align == nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_UNKNOWN)
>+    align = nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_XMIDYMID;
>+  if (meetOrSlice == nsIDOMSVGPreserveAspectRatio::SVG_MEETORSLICE_UNKNOWN)
>+    meetOrSlice = nsIDOMSVGPreserveAspectRatio::SVG_MEETORSLICE_MEET;

Worth porting bug 328348 first? I don't suppose it matters for a branch patch.

r=longsonr

Comment 12

[tor]

(In reply to comment #11)
> (From update of attachment 242220 [details] [diff] [review] [edit])
> I don't know whether GDI+ still compiles/runs on the branch. It won't after
> this with the cairo code directly in nsSVGUtils.cpp. I assume you don't regard
> this as a problem.

Yeah, GDI+ is dead to us.

> >+already_AddRefed<nsIDOMSVGMatrix>
> >+nsSVGImageFrame::GetImageTransform()
> >+{
> 
> >+  // default to the defaults
> >+  if (align == nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_UNKNOWN)
> >+    align = nsIDOMSVGPreserveAspectRatio::SVG_PRESERVEASPECTRATIO_XMIDYMID;
> >+  if (meetOrSlice == nsIDOMSVGPreserveAspectRatio::SVG_MEETORSLICE_UNKNOWN)
> >+    meetOrSlice = nsIDOMSVGPreserveAspectRatio::SVG_MEETORSLICE_MEET;
> 
> Worth porting bug 328348 first? I don't suppose it matters for a branch patch.

That did cross my mind when I unexpectedly ran across that viewport code.  Figured that we'd want to keep a branch patch as small in scope as is reasonable, though.

Comment 13

[tor]

Comment on attachment 242220 [details] [diff] [review]
backported branch patch

Specification compliance issue, similar fix has been on the trunk since September 19th.

Comment 14

[Daniel Veditz [:dveditz]]

Comment on attachment 242220 [details] [diff] [review]
backported branch patch

Need to wait for next release.

Comment 15

[Jay Patel [:jay]]

Comment on attachment 242220 [details] [diff] [review]
backported branch patch

Approved for 1.8 branch, a=jay for drivers.

Comment 16

[tor]

Checked in on MOZILLA_1_8_BRANCH.

Comment 17

[Carsten Book [:Tomcat]]

Verified Fixed for 1.8.1.2 with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/20070108 BonEcho/2.0.0.2pre ID:2007010804