1119698 - Mouse events firing on hidden <image> elements

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[atmd1983]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150106233618

Steps to reproduce:

JSbin created to replicate

http://jsbin.com/focixu
please also see the stackoverflow question with more details.

Summary:

I have an svg' with visibility:hidden and mouse events bound to it.
The elements are invisible as per the css, however on mouse over and click those bound events are still firing.


Actual results:

both mouse over and click events fired on the hidden elements.
This only effects the svg <image> tag


Expected results:

The events should not fire, the elements are hidden and should be inactive

N.B. I have tested this in firefox 31 and the issue doesn't exist, however it DOES exist in firefox version 34, 35 beta and 36 dev edition.

Comment 1

[atmd1983]

Related stackoverflow question

http://stackoverflow.com/questions/27844884/d3-events-firing-on-a-hidden-svg-element

Comment 2

[Robert Longson [:longsonr]]

Attachment: image.txt

Comment 3

[Robert Longson [:longsonr]]

Also http://stackoverflow.com/questions/27949792/click-events-propagating-to-hidden-svg-images-with-pointer-events-visible-in-f

Comment 4

[Jonathan Watt [:jwatt]]

Comment on attachment 8546697 [details] [diff] [review]
image.txt

Review of attachment 8546697 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/svg/nsSVGImageFrame.cpp
@@ +411,5 @@
>  nsSVGImageFrame::GetFrameForPoint(const gfxPoint& aPoint)
>  {
> +  if (!(GetStateBits() & NS_STATE_SVG_CLIPPATH_CHILD) && !GetHitTestFlags()) {
> +    return nullptr;
> +  }

I don't follow this logic. Why the requirement that the frame is not NS_STATE_SVG_CLIPPATH_CHILD?

Comment 5

[Robert Longson [:longsonr]]

path has that logic.

http://mxr.mozilla.org/mozilla-central/source/layout/svg/nsSVGPathGeometryFrame.cpp#286

Though as far as I can tell text does not (though perhaps it should)

I imagine its to make PointIsInsideClipPath return whether or not we're in the clip path. So basically we don't support setting pointer-events on individual elements within clip-paths.

Comment 6

[Robert Longson [:longsonr]]

Before bug 1049256 images called the path code for hit testing so all I'm doing is putting it back the way it was. We can have a follow up to determine what the right answer is for clip-paths if you want.

Comment 7

[Jonathan Watt [:jwatt]]

Comment on attachment 8546697 [details] [diff] [review]
image.txt

This function still seems like a bit of a mess to me, but this does fix the bug.

Comment 8

[Jonathan Watt [:jwatt]]

Can you add an XXX/TODO comment noting that we should align with the nsSVGPathGeometryFrame code with regards to clipPath and the hit testing flags though?

Comment 9

[Jonathan Watt [:jwatt]]

Actually just land this as-is and I'll file another bug for the other changes that I want to make.

Comment 10

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db7b76dfb89d

Comment 11

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2bfd9d55f94c

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2bfd9d55f94c

Comment 13

[Robert Longson [:longsonr]]

Comment on attachment 8546697 [details] [diff] [review]
image.txt

Approval Request Comment
[Feature/regressing bug #]: bug 1049256
[User impact if declined]: hidden SVG images receive mouse events when they should not. There are a couple of stackoverflow questions about this.
[Describe test coverage new/current, TBPL]: includes a mochitest
[Risks and why]: pretty small, basically reinstates the codepath removed in bug 1049256
[String/UUID change made/needed]: none

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/140515481763
https://hg.mozilla.org/releases/mozilla-beta/rev/94e7cb795a05