My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 111: Implement removeEventListener
2 people starred this issue and may be notified of changes. Back to list
Status:  Fixed
Owner:  bradneub...@gmail.com
Closed:  Apr 2010


Sign in to add a comment
 
Project Member Reported by bradneub...@gmail.com, Jun 10, 2009
I've implemented all the event handling stuff and it works for our samples.
However, I haven't done removeEventListener yet, which I will do once on
trunk and once I have a large suite of unit tests for event handling.
Sep 24, 2009
#1 Duffy...@gmail.com
I can provide samples for unit tests when you start to work on this issue.
Sep 24, 2009
Project Member #2 bradneub...@gmail.com
Thanks Duffy; can you attach the unit tests and samples to this bug soon?
Sep 24, 2009
#3 Duffy...@gmail.com
Brad

I have attached two files.  I am creating an dynamic map generation tool for campgrounds.  We hope to release 
it into production the 1st of Nov.  Each time the user updates a search criteria, the map will automatically 
show the sites available.  You can test it out by alternating which site group is available with the buttons on 
top.  The events being removed from the map are the mouseover events.

removeEventListener is working in native svg on Safari and Firefox.    Force flash fails by not removing the 
mouseover events once the next site group has been selected.

I hope this helps....  Drop me a note if you have any questions.
removeEventListener_test.html
3.4 KB   View   Download
map_scoured.svg
21.6 KB   View   Download
Sep 24, 2009
Project Member #4 bradneub...@gmail.com
Thanks Duffy, this is great. I'm hoping to hit removeEventListener in October so it
should be in there by Nov 1st.
Oct 19, 2009
Project Member #5 bradneub...@gmail.com
(No comment was entered for this change.)
Labels: -Priority-Medium Priority-Critical
Nov 12, 2009
#6 gregorio...@gmail.com
Hi, I needed to remove an event, so I implemented removeEventListener. I'm attaching
two patch, one is for the svg.js, the other one is for the swf. I tested it and it is
working fine, the test-case attached in this issue by Duffy works too. 

Anyway, I didn't implement the removing of keydown event listeners, because I had no
test-case to try it on. The blocks_game sample doesn't work in FF when rendering with
Flash (it doesn't catch any keyboard event). Is it a knows issue?
svg-uncompressed.js.patch
856 bytes   View   Download
SVGViewerWeb.as.patch
3.3 KB   View   Download
Nov 12, 2009
Project Member #7 bradneub...@gmail.com
This is awesome! In terms of the key stuff, I'm thinking of removing that in general
since it can probably mess with the page and people should just be adding key
listeners on the window itself.

Where were you able to QA the patch?

Some small feedback looking at the patch:

!     if (this._listeners[type] === undefined) {
!         return;
!     }

For more compact code, I would use if (!this._listeners[type])

!     
!     this._listeners[type].pop({ type: type, listener: listener, 
!                                  useCapture: useCapture });
!     this._listeners[type]['_' + listener.toString() + ':' + useCapture] = null;
!     
!     
!     //TODO implement the removing of keydown event listener
!     this._handler.sendToFlash('jsRemoveEventListener', [ this._guid, type ]);

Other conditions I would make sure to handle:

* Does removeEventListener work on disconnected nodes?
* Does it work on SVG roots?
* Does it work tunneling into an SVG OBJECT and removing an event listener?
* If I have multiple event listeners on a single element and I remove just one, do
the others stay present?

BTW, when doing these kinds of patches I like to also update the unit tests in
tests/browser-tests. The primary code is in test_js.js, which is pulled in by
test_js1.html and test_js2.html. The two HTML files pull in test_js.js and use it in
different contexts. One HTML file uses direct embedded SVG and the other one uses SVG
OBJECTs. test_js.js is a bit sprawling and is written to be a little bit generic
since it can be pulled into two contexts.

For removeEventListener, I would create a testRemoveEventListener method with a bunch
of unit tests. I generally also add a console.log statement if there is a some
behavior or visual behavior that should be confirmed that an assertion can't affirm.

Want to take a stab at this? I should document it better; feel free to pepper me with
questions.

Reviewing the SVGViewerWeb patch, it looks like you ran the diff in reverse so all
the new lines have minuses. Things look good. However, I don't see the implementation
of removeEventListener over on the ActionScript side; does that live on SVGNode.as?

Thanks again!
Nov 12, 2009
Project Member #8 bradneub...@gmail.com
BTW, also, it's good to try to QA on the following platforms:

* FF 3+ (3 or 3.5)
* Safari 4+
* IE 6/7/8

I like to run through the unit tests in tests/browser-tests/test_js1.html and
test_js2.html. I add the flag ?svg.render.forceflash=true and false to make sure
things work on the native and Flash renderers (the unit tests help a huge amount with
this stuff, especially edge conditions). Its good to also test samples/demo.html. I
don't always run through all this stuff but in general I do when doing a major change
as this stuff can get tricky quickly.
Nov 13, 2009
#9 gregorio...@gmail.com
I didn't notice the minuses in the patch, I'm attaching the correct one.

The test I've done are on a svg I use at work and on the Duffy's test case. I can
update unit tests too. I've quickly had a look at them, I'll try to do something
during this weekend, including making some other test to answer your 4 questions.

Anyway, I tested the patch with my test case on FF 3.5, Safari 4 and IE6.

For the implementation of removeEventListener on the ActionScript side: I call it on
the topSprite of a SVGNode. The topSprite is a flash.display.Sprite, that inherits
the removeEventListener (and addEventListener too) from EventDispatcher.
SVGViewerWeb.as.patch
3.3 KB   View   Download
Nov 19, 2009
Project Member #10 bradneub...@gmail.com
(No comment was entered for this change.)
Labels: Patch
Apr 8, 2010
Project Member #11 bradneub...@gmail.com
I'm working on this bug now. Note that there are a number of bugs unrelated to this in the test case 
that @Duffy.Ty gave above (removeEventListener_test.html):

* In your OBJECT tag in your HTML, you must use the classid attribute for IE (see 
http://codinginparadise.org/projects/svgweb/docs/QuickStart.html#object_tag for details).
* You use indexOf() on a JavaScript Array, which is not supported on IE.
Status: Started
Apr 8, 2010
Project Member #12 bradneub...@gmail.com
This issue was closed by revision r1102.
Status: Fixed
Apr 8, 2010
Project Member #13 bradneub...@gmail.com
I adapted @Duffy.Tys tests and put them into tests/browser-tests/issue-tests/ issue111 .html and 
 issue111 .svg
Sign in to add a comment

Powered by Google Project Hosting