302103 - implement SVG events

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jonathan Watt [:jwatt]]

Implementing the SVGScroll and SVGZoom events is essential for scale invariant
features, such as the controls, label text and tacks in google maps for example.
I think it's important we implement this for Firefox 1.5.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: snapshot

Here's a snapshot of what I've got so far. Hopefully it's fairly close, but
event handlers don't get called for some reason.

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: updated snapshot

Here's an updated snapshot. The new events are now being dispatched correctly
and they're reaching the handlers. I've still got to figure out how to
implement the 'onzoom' and 'onscroll' attributes though. This is tricky because
the event attribute names don't match up with the names of the events (i.e.
'SVGZoom' and 'SVGScroll').

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: snapshot - 'onzoom' and 'onscroll' attributes now work

Comment 4

[Jonathan Watt [:jwatt]]

Appart from the reasons I gave in comment 0, I think it's important to fix this
now because we don't want things like

  svgDocEl.addEventListener('load', loadHandler, false);

to be what works in Firefox 1.5, but then

  svgDocEl.addEventListener('SVGLoad', loadHandler, false);

to be what works in later releases.

Comment 5

[Jonathan Watt [:jwatt]]

Attachment: latest patch

Comment 6

[Jonathan Watt [:jwatt]]

Attachment: patch for dispatching SVGLoad, SVGScroll and SVGZoom

Links to the relevant parts of the spec are given below. Look at these points
and below.

http://www.w3.org/TR/SVG/script.html#InterfaceSVGEvent
http://www.w3.org/TR/SVG/interact.html#LoadEvent
http://www.w3.org/TR/SVG/interact.html#EnableZoomAndPanControls

Comment 7

[Jonathan Watt [:jwatt]]

Attachment: TESTCASE

Comment 8

[tor]

(In reply to comment #6)
> Created an attachment (id=192076) [edit]
> patch for dispatching SVGLoad, SVGScroll and SVGZoom

nsDOMSVGZoomEvent::GetZoomRectScreen should be "implemented" with
NS_NOTYETIMPLEMENTED and NS_ERROR_NOT_IMPLEMENTED.

In nsSVGSVGElement, I'd like to see CurrentScale and CurrentTranslate use the
same mechanism for recording the current translation/scale.  The method you're
using for scale now is clearly commented, but I don't think it saves enough to
make it worthwhile.

Pan/zoom will often change both scale and translate at once, which will send
multiple events with the current path.

Comment 9

[Jonathan Watt [:jwatt]]

Attachment: patch to address tor's comments

This patch adds two methods to nsISVGSVGElement to allow zoom and pan controls
to modify currentScale *and* currentTranslate in a way that will only dispatch
one DOM event.

Comment 10

[Jonathan Watt [:jwatt]]

Attachment: patch updated to address off-bug comments

Comment 11

[Jonathan Watt [:jwatt]]

Attachment: now initialising mZooming

Comment 12

[tor]

Comment on attachment 192385 [details] [diff] [review]
now initialising mZooming

>--- mozilla/content/svg/content/src/nsSVGSVGElement.cpp	19 Feb 2005 10:31:24 -0000	1.50
>+++ mozilla/content/svg/content/src/nsSVGSVGElement.cpp	11 Aug 2005 15:50:32 -0000
>+  NS_IMETHOD_(float) GetPreviousTranslate_x();
>+  NS_IMETHOD_(float) GetPreviousTranslate_y();
>+  float                             mPreviousTranslate_x;
>+  float                             mPreviousTranslate_y;

These don't really follow the mozilla naming convention.

Rest looks ok, though why the w3c called the currentTranslate event "SVGScroll"
is a bit of a mystery.

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 192385 [details] [diff] [review]
now initialising mZooming

sr=jst

Comment 14

[Holger Will]

Attachment: another testcase

you will need the zoom and pan extension installed to test this...

Comment 15

[Holger Will]

from a first look at this, i'd say this works perfect, but i will do more
extensive testing.
great job jonathan, thanks a lot.

Comment 16

[Asa Dotzler [:asa]]

Please land this on the trunk and after it's verified we'll evaluate for branch
inclusion.

Comment 17

[Jonathan Watt [:jwatt]]

Asa, I already landed on the trunk on 2005-08-25 14:31 PST as per shaver's
request. We've requested testing of the trunk nightlies via tor's blog.

Comment 18

[Boris Zbarsky [:bzbarsky]]

If it's fixed on trunk it should be marked fixed (and get the fixed1.8 keyword
when it's fixed on branch).

Comment 19

[Jonathan Watt [:jwatt]]

Anyone want to verify this is fixed on the trunk so we can land it on the branch?

Comment 20

[Bruce Rindahl]

Attached test case does not show text.

Comment 21

[Bruce Rindahl]

Attachment: textPath test

Attached test case

Comment 22

[Jonathan Watt [:jwatt]]

Comment on attachment 194275 [details]
textPath test

Hi Bruce. Thanks for the report, and especially for such a small testcase!
Little testcases like that help a lot. Unfortunately you've attached to the
wrong bug. Can you attach your testcase to
https://bugzilla.mozilla.org/show_bug.cgi?id=282579 please?

Comment 23

[Holger Will]

verifying fix, 
one thing though, in comment #9 you say there is a method to change
currentTranslate and currentScale so the changes fire only one event.
what is this method, how do i use it ?
beside that it works great, i would say land it ASAP.

Comment 24

[Jonathan Watt [:jwatt]]

Holger, can you select "Mark bug as VERIFIED" to properly set the bug status.

Unfortunately the method you mention is only available to internal code. Your
extension can't use it I'm afraid.

Comment 25

[Holger Will]

unfortunatly, it seems i cant change the status of a bug, sorry.

Comment 26

[Asa Dotzler [:asa]]

Comment on attachment 192385 [details] [diff] [review]
now initialising mZooming

approved. holger, I updated your account to be able to resolve bugs.

Comment 27

[Jonathan Watt [:jwatt]]

checked into trunk

Comment 28

[Jonathan Watt [:jwatt]]

err, I mean branch

Comment 29

[Johanne Jean-Baptiste]

Attachment: Test Result: Zoom and Pan

When zooming in repeatedly (more than 10 times), the svg rendered becomes
bizarre.
When zooming out, after about 6th time, memory is guzzled up, after 10th time,
Browser stops responding.  I would recommend having a maximum number of zoom
ins and zoom outs.

Johanne

Comment 30

[Jonathan Watt [:jwatt]]

Hi Johanne. :-) Oops, I just remembered I forgot to reply to your email. (I'll
get to that next.) Regarding your suggestion, you're quite right. I found the
same problem when messing with

  http://jwatt.org/svg/tests/zoom-and-pan-controls.svg

I've opened bug 308734 for this issue.

Comment 31

[Hardeep]

Hello :) So I thought I would give a little debriefing on what I am currently implementing. As of right now I am implementing the zoomEventListener as well implementing controls and events to do native zooming and panning. I thought I would either implement a new listener for these events or use the current svg events listener.

In my point of view a seperate listener would be better :) any suggestions are welcome.

Comment 32

[Jonathan Watt [:jwatt]]

Hi Hardeep. This bug was closed, fixed, about 3 years ago. Can you open a new bug report and start over there? I only pointed you to this bug since I thought you might find in useful to review the comments and patches here. Sorry if that wasn't clear.