828815 - SVGRectElement["onxxx"] = Function does not work, No Tooltip on SVG graphic (StatCounter)

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Markus Popp]

Access http://gs.statcounter.com/#browser-ww-daily-20130109-20130109-bar with Firefox 20 or 21, with Flash disabled.

There should be a tooltip on each (SVG) bar in the graph to show the percentage. While up to Firefox 19 this works, in Firefox 20 and 21 there is no tooltip.

Comment 1

[Justin Dolske [:Dolske]]

WFM: On a current OS X nightly, I get a tooltip on that graph, showing the name of the browser and a percentage.

I don't see any SVG here, though. If I right-click on the graph, I get a context menu for Flash (and the "tooltips" are clearly not browser tooltips).

Comment 2

[Markus Popp]

Flash needs to be disabled, then an SVG version will show up instead.

Comment 3

[Matthias Versen [:Matti]]

Last good nightly: 2013-01-06
First bad nightly: 2013-01-07

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=20d1a5916ef6&tochange=605ae260b7c8

I guess that this is caused by one of David's checkins and I would bisect the exact changeset if required

Comment 4

[Matthias Versen [:Matti]]

The first bad revision is:
changeset:   117786:04feda337f8b
user:        David Zbarsky <[e-mail]>
date:        Sun Jan 06 04:32:01 2013 -0500
summary:     Bug 825147: Convert SVGRectElement to WebIDL r=bz

Comment 5

[Boris Zbarsky [:bzbarsky]]

Matti, thanks!

Comment 6

[Robert Longson [:longsonr]]

FWIW I don't think the tooltips are created by having <title> children. I think they are programatically generated.

There is a TypeError: src is null in the Error Console. That may be where the problem is perhaps?

Comment 7

[Masatoshi Kimura [:emk]]

TypeError is generated even on Firefox 18. I suppose it's unrelated to the tooltip problem.

Comment 8

[David Zbarsky (:dzbarsky)]

The tooltips appear if I disable the new bindings for SVGRectElement.  Unfortunately, I haven't been able to figure out how to save the site locally and still have it work so I can't reduce it.  Maybe the tooltips are broken because event handlers on new binding nodes do something different?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Event handlers on new-binding nodes shouldn't do anything different...

Alice0775, do you think you can make a more-reduced testcase here?

Comment 10

[Robert Longson [:longsonr]]

The chart seems to be based on highcharts. Highcharts have a demo site and that seems to have the same issue i.e. no tooltips on trunk:

http://www.highcharts.com/demo/column-basic

Since this includes a way to copy to jsfiddle perhaps that will be more amenable to debugging/reduction.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Yes, that should be way simpler to reduce.  Thanks!

Comment 12

[Alice0775 White]

Attachment: testcase html

Color should change by mouse hover.

Comment 13

[Alice0775 White]

in highcharts.js,

    on: function (a, b) {
      if (Ba && a === "click") this.element.ontouchstart = function (a) {
        a.preventDefault();
        b()
      };
      this.element["on" + a] = b;
      return this
    },


|this.element["on" + a] = b;|  does not work  in Aurora20.0a1 and Nightly21.0a1,  but works in Firefox19.0beta and earlier

Comment 14

[Alice0775 White]

Attachment: testcase html

Color should change by mouse hover.

Comment 15

[Masatoshi Kimura [:emk]]

onxxx attributes are only available on HTML elements. Evang matter.

Comment 16

[Alice0775 White]

Attachment: onmouseover="xxxx" works

(In reply to Masatoshi Kimura [:emk] from comment #15)
> onxxx attributes are only available on HTML elements. Evang matter.

However this onmouseover="mouseover_colorsize(evt)" attributes of SVGRectElement  works in Nightly21.0a1.

What is going on?

Comment 17

[Jonathan Watt [:jwatt]]

Setting |svgElement.onxxx = func| used to work, and for backwards compatibility and consistency with HTML I think it should continue to work.

Comment 18

[Jonathan Watt [:jwatt]]

(In reply to Alice0775 White from comment #16)
> What is going on?

Sounds like setting of the JS property on SVG elements isn't being mapped to setting the attribute/event handler.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Alice0775, thank you for the testcase!

> Setting |svgElement.onxxx = func| used to work, and for backwards compatibility and
> consistency with HTML I think it should continue to work.

That's trivial enough to do in our WebIDL code.  Can we get this specced on the SVG end?  Right now it's certainly not in the spec...

The reason this used to work is that DOM_CLASSINFO_SVG_ELEMENT_MAP_ENTRIES includes nsIInlineEventHandlers and nsITouchEventReceiver, but that stuff is not present on SVGElement in the WebIDL.

David, you want to fix this?  You'll need to do the prefcontrolled hackery for touch events that HTMLElement does...  On Aurora this will probably just need copy/paste.  On trunk, bug 829072 will make things nicer here eventually, but you can do copy/paste for now.

Comment 20

[David Zbarsky (:dzbarsky)]

Attachment: Patch

This fixes the testcase.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 702470 [details] [diff] [review]
Patch

You need to make the interface [PrefControlled] and have SVGElement::PrefEnabled() call nsGenericHTMLElement::PrefEnabled().  Otherwise the touch stuff won't work right.

r=me with that change, assuming you just copied that list from HTMLElement.

Comment 22

[David Zbarsky (:dzbarsky)]

Attachment: Patch for checkin

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 823394
User impact if declined: Websites that set event handlers on svg elements from script may not work
Testing completed (on m-c, etc.):
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: None

Comment 23

[David Zbarsky (:dzbarsky)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d21cb6921b71

Comment 24

[Cameron McCormack (:heycam)]

We have already resolved to include event listener IDL attributes in SVG 2:

  https://svgwg.org/svg2-draft/interact.html#SVGEvents (annotation 2 there)

so it should happen in due course.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Sounds fine.  Note that HTML has moved these out into interfaces it pulls in via "implements", so SVG could just use those.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Oh, and.... this bug needs a test!

Comment 27

[Cameron McCormack (:heycam)]

(In reply to Boris Zbarsky (:bz) from comment #25)
> Sounds fine.  Note that HTML has moved these out into interfaces it pulls in
> via "implements", so SVG could just use those.

Great!

Comment 28

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 1 merged to

Comment 29

[Boris Zbarsky [:bzbarsky]]

Attachment: Part 2 merged to

Comment 30

[Boris Zbarsky [:bzbarsky]]

Attachment: Merged to

Comment 31

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 702646 [details]
Part 1 merged to

Wrong bug.  :(

Comment 32

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d21cb6921b71

Comment 33

[bhavana bajaj [:bajaj]]

Comment on attachment 702482 [details] [diff] [review]
Patch for checkin

low risk and early in aurora so approving.

Comment 34

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/d6a4739c5fac

Comment 35

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (X11; Linux x86_64; rv:20.0) Gecko/20100101 Firefox/20.0
Mozilla/5.0 (Windows NT 6.2; WOW64; rv:20.0) Gecko/20100101 Firefox/20.0

Verified as fixed in Firefox 20 beta 6 (buildID: 20130320062118) using provided test case.

Comment 36

[Cornel Ionce [:noni] [Hubs QA]]

Verified as fixed in Firefox 21 beta 2 (build ID: 20130401192816), using the attached test case.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (Windows NT 6.2; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0
Mozilla/5.0 (X11; Linux x86_64; rv:21.0) Gecko/20100101 Firefox/21.0