619955 - pointer-events is fairly broken on SVG text

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Cameron McCormack (:heycam)]

Moving the mouse over a <text pointer-events="fill" fill="none"> element should dispatch mouse events.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Test

Comment 2

[Jonathan Watt [:jwatt]]

Actually our handling of pointer-events on text is pretty broken. nsSVGGlyphFrame::GetFrameForPoint requires that the pointer event be inside mRect:

http://hg.mozilla.org/mozilla-central/annotate/b9e2c6e0f2ad/layout/svg/base/src/nsSVGGlyphFrame.cpp#l413

However, when nsSVGGlyphFrame::UpdateCoveredRegion sets mRect it does not account for stroke if stroke-opacity=0, and if we have no stroke and fill=none, then mRect is actually set to empty (the cause of the problem in comment 0):

http://hg.mozilla.org/mozilla-central/annotate/b9e2c6e0f2ad/layout/svg/base/src/nsSVGGlyphFrame.cpp#l468

Besides that, nsSVGGlyphFrame::ContainsPoint (called by GetFrameForPoint) only checks if the hit is in the fill, so any stroke that lies outside the character cell will never intercept events.

http://hg.mozilla.org/mozilla-central/annotate/b9e2c6e0f2ad/layout/svg/base/src/nsSVGGlyphFrame.cpp#l1476

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: patch

There's not too much in the way or real changes here. Essentially I'm just moving things about, with the few real changes being located in nsSVGGlyphFrame.

Comment 4

[Jonathan Watt [:jwatt]]

Actually, the HITTEST_MASK_CHECK_MRECT check needs to be removed because mRect is not based on character cells, but on actual fill/stroke areas.

Comment 5

[Robert Longson [:longsonr]]

I'd really like to see the svg image element tested as it has some different requirements to everything else i.e. fill and stroke properties don't apply, however image frames are derived from nsSVGPathGeometry frames. The code changes seem basically OK.

Comment 6

[Jonathan Watt [:jwatt]]

Agreed, but this bug is about <text>, and I've only just finished working out <text>'s special code and test requirements. I'll get to <image> in bug 311942, but right now it's too broken to just add to the test framework in test_pointer-events.xhtml.

Comment 7

[Jonathan Watt [:jwatt]]

Comment on attachment 507410 [details] [diff] [review]
patch

longsonr: thanks!

Passed Try fine: http://tbpl.mozilla.org/?tree=MozillaTry&rev=7952468eded0

Requesting approval.

Comment 8

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

Comment on attachment 507410 [details] [diff] [review]
patch

+  if (mask & HITTEST_MASK_FILL || mask & HITTEST_MASK_STROKE) {

if (mask & (HITTEST_MASK_FILL | HITTEST_MASK_STROKE))

Comment 9

[Jonathan Watt [:jwatt]]

Thanks.

Pushed http://hg.mozilla.org/mozilla-central/rev/822a61861a38