Letractively / svgweb

Automatically exported from code.google.com/p/svgweb
Other
0 stars 0 forks source link

Problem with using svgweb and jquery together #375

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Svgweb and jquery seem to conflict. In the attached example script, I try to 
insert an svg document, while 
jquery is also loaded. When the flash renderer is forced, firebug shows several 
errors (see attached screenshot) 
and doesn't display the svg.

To reproduce the problem:
1. Copy test-jquery.html and test.svg to a directory on your webserver
2. Download jquery-1.3.2 from http://jquery.com/ and place it in the same 
directory as test-jquery.html
3. Place the svgweb files in the same directory as test-jquery.html (or change 
the path in the file)
4. Load the page in firefox

Note that if I don't force the flash renderer, the svg displays fine. I still 
get the message "Error while 
firing onload: event is undefined".

While googling for this problem, I found this blog post, which looks like a 
similar problem:
http://antimatter15.com/wp/2009/08/totally-failed-mini-project-svg-edit-svgweb/

Original issue reported on code.google.com by thomaskelder on 2 Nov 2009 at 3:14

Attachments:

GoogleCodeExporter commented 8 years ago
jQuery registers events using window.addEventListener, which is redefined by 
svgweb.
I modified _interceptOnloadListeners() to ovverride window.addEventListener
intercepting two new events, "svgload" and "onsvgload", that aren't used by 
jQuery,
and any other JS framework either. 

Obviously, I had to modify every call that registers a "load" or "onload" event,
changing them according with the two new events. It seems to be working 
perfectly
now. I'm attaching the modified svg-uncompressed.js. 

Original comment by gregorio...@gmail.com on 3 Nov 2009 at 2:08

Attachments:

GoogleCodeExporter commented 8 years ago
Interesting fix; can you actually generate a patch file with the differences and
attach it to this bug? Were you able to QA it on Internet Explorer and 
FF/Safari?

BTW, in your original page, did you have svg.js as the first script file 
loaded? That
is required for SVG Web; perhaps that would have made it work with JQuery.

Can you create a reduced test case using JQuery and SVG Web and attach it to 
this
bug? That will help me address this quicker.

Original comment by bradneub...@gmail.com on 3 Nov 2009 at 4:03

GoogleCodeExporter commented 8 years ago
Yes I can generate a patch, but I'm trying to make it works in IE too first. It 
works
nice in FF and Safari, but not in IE, and I'm working on it. 

The svg.js is the first script loaded, anyways, and it generated errors with 
jQuery
before my patch.

I'll attach the recuced test case when I get IE works too ;)

Original comment by gregorio...@gmail.com on 3 Nov 2009 at 4:54

GoogleCodeExporter commented 8 years ago
Well, I worked a lot trying to understand why IE gives me errors sometimes, but 
I
still can't figure out what is the problem. 

I'm attaching the patch, anyway, and a modified helloworld.html. Use it instead 
of
the helloworld.html which comes with the svgweb release. When you open with FF,
firebug gives you an error while firing the load event. Using my patch the error
doesn't happen. This test-case works fine in FF, Safari and IE.

Unfortunately, using the patch in other sample-pages, it gives you other errors,
sometimes in FF and sometimes in IE. I noticed that this happens when in the 
page
there is a redefinition of window.onload. I'm pretty sure that jQuery (and maybe
every other JS framework) define some events on window.onload, so it could be a
problem to redefine that while using jQuery.

Since the modified helloworld.html that I'm attaching still works fine even
redefining window.onload, it could be some piece of code in other pages that 
gives
some problems. I'll keep studing it, anyway.

Original comment by gregorio...@gmail.com on 4 Nov 2009 at 9:54

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for the patch! I can confirm it works on your helloworld example in 
firefox. However, when I try it in 
my own example, firebug shows me this error and the image doesn't show:

uncaught exception: [Exception... "Illegal operation on WrappedNative prototype 
object" nsresult: "0x8057000c 
(NS_ERROR_XPC_BAD_OP_ON_WN_PROTO)" location: "JS frame :: 
http://localhost/wikipathways/wpi/extensions/PathwayViewer/lib/svgweb/svg-uncomp
ressed.js :: anonymous :: line 
2415" data: no]

This also happens when I run the demo page distributed with svgweb. Both have 
in common that they dynamically 
add the <object> element by a javascript. In helloworld.html, the <object> 
element is already present in the 
html. Perhaps the problem lies in that part.

In the meantime, I found another workaround to solve the jQuery conflict by 
setting the jQuery.event.triggered 
to true. This prevents jQuery from messing with the event that is triggered by 
svgweb. See the attached patch 
for the changes. I realize that this not the most elegant solution, but it 
might be useful until a more 
general solution is found.

Original comment by thomaskelder on 4 Nov 2009 at 11:19

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks Thomas for the patch and for the advices, they gave me an hint on what I 
have
to look for. I'm pretty sure that svgweb redefine something that will be 
redefined
also by jQuery, so it would be better to find a way to redefine stuffs without
messing up with js frameworks in general.

Original comment by gregorio...@gmail.com on 4 Nov 2009 at 11:32

GoogleCodeExporter commented 8 years ago
I found the problem, finally. Now I get svgweb working together with jQuery 
without
any error in FF, Safari and IE. I'm attaching the new (second) patch, apply it 
after
the first. Brad, please let me know if you prefere a unique patch.

Also, it would be better to test svgweb with other frameworks, such as mootools,
prototypejs, ext.js, dojo... They should already work fine with my patch, but 
the
tests would tell us it. Brad, is it ok to attach here the tests for the others
frameworks, or is it better to open a new issue for each of them?

Original comment by gregorio...@gmail.com on 5 Nov 2009 at 8:13

Attachments:

GoogleCodeExporter commented 8 years ago
I can confirm it works after applying your two patches (tested in FF and IE). 
Thank 
you very much for fixing this!

Original comment by thomaskelder on 5 Nov 2009 at 8:32

GoogleCodeExporter commented 8 years ago
Thanks for doing this!

Can you consolidate the patch into a single patch I can apply?

Also, I think its better to open new issues for other individual toolkits we 
might
have issues with, such as "Make sure SVG Web and Dojo work together" for 
example,
perhaps with a reference to Issue 375.

Original comment by bradneub...@gmail.com on 5 Nov 2009 at 8:20

GoogleCodeExporter commented 8 years ago
Here is the single patch.

I'll open new issues as soon as possible and attach test-cases for the other 
most
used toolkits.

Original comment by gregorio...@gmail.com on 6 Nov 2009 at 8:21

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks Gregorio!

Original comment by bradneub...@gmail.com on 6 Nov 2009 at 9:13

GoogleCodeExporter commented 8 years ago

Original comment by bradneub...@gmail.com on 19 Nov 2009 at 9:48

GoogleCodeExporter commented 8 years ago

Original comment by bradneub...@gmail.com on 20 Nov 2009 at 10:52

GoogleCodeExporter commented 8 years ago
based on Gregorio's Nov 06, I created a new patch against r1036. See attached.

Original comment by rdwo...@gmail.com on 21 Dec 2009 at 4:26

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for the patch. The root of the problem is that patch takes away being 
able to
subscribe to the onload event and introduces a new event, SVGLoad. I'm not 
against
doing this, I've just been mulling over whether it makes sense to break 
backwards
compatibility here and have a new event; whether to just go back to the (much 
older)
svgweb.addOnLoad() system; or whether to figure out whether there is a way to 
keep
the window.onload abstraction and magic without breaking JQuery and other 
libraries.
I'm not sure yet what I'm going to do.

Original comment by bradneub...@gmail.com on 22 Dec 2009 at 10:46

GoogleCodeExporter commented 8 years ago
I hate to ask but could you possibly give a rough estimate/forecast of how long 
you reckon this issue might 
take to end up resolved? Is it something very complex? 

I don't want to sound demanding and I really appreciate the work, but I need to 
make a decision about 
whether to use SVG in an upcoming project and really love to use svgweb - but 
jQuery compatibility is a 
necessity. So just trying to kind of get an idea of what the options are. 
Obviously the idea of being able to 
reliably use SVG on IE is very compelling.

Thanks and sorry to be "that guy" ....

Original comment by sho.fukamachi@gmail.com on 7 Jan 2010 at 2:55

GoogleCodeExporter commented 8 years ago
I've been watching this issue since it was posted as it is an absolute 
showstopper
which prevents us from switching some graphics to svg. I've got a seperate 
branch on
my main project with a 'mostly working' svg implementation waiting to go -- 
provided
svgweb and jquery play nice. With this issue outstanding, developers have to 
make the
choice between svgweb and jquery, and I'm afraid svgweb will lose in my case 
(and
probably most projects already invested in the awesomeness that is jquery).

I also don't want to join Sho in being 'that guy' but..

Original comment by ivar%oob...@gtempaccount.com on 20 Jan 2010 at 6:54

GoogleCodeExporter commented 8 years ago
@sho.fukamachi & ivar: In practice, the patch attached to this issue seems to 
works 
fine, so maybe it's an idea to use the patched version of svgweb for your 
project until 
this issue has been fixed in a better way? Just apply the patch to the latest 
release. 
So far, I haven't experienced any problems with this approach in my project 
that uses 
svgweb.

Original comment by thomaskelder on 22 Jan 2010 at 9:14

GoogleCodeExporter commented 8 years ago
Thanks for that information Thomas. Great to hear confirmation that someone's 
using it without issues. Will give 
it a shot!

For people's convenience I attach the patched svg.js file.

Original comment by sho.fukamachi@gmail.com on 22 Jan 2010 at 9:44

Attachments:

GoogleCodeExporter commented 8 years ago
Hi everyone, sorry for the delay. I haven't had as much time to work on SVG Web 
lately. 
This is an important bug that I want to get to but there might be some more 
delay since 
I'm gearing up in a new position at work. 

In terms of the patches, I'll probably go this route and get rid of the 
window.onload 
magic I have now. This will be a breaking change but its probably safer in 
general to do.

Original comment by bradneub...@gmail.com on 5 Feb 2010 at 4:37

GoogleCodeExporter commented 8 years ago
Hi everyone, I tried to analyze the thoughts of Brad, and made a new patch 
based on
his considerations.
Brad, probably you'll modify something to make it better, but the logic behind 
the
patch is pretty simple. Let me know if it is ok with this new approch.

Original comment by gregorio...@gmail.com on 6 Feb 2010 at 11:48

Attachments:

GoogleCodeExporter commented 8 years ago
I have no idea how to install a patch. For now, IE can just display a PNG file.

Original comment by keith.ty...@gmail.com on 14 Feb 2010 at 12:28

GoogleCodeExporter commented 8 years ago
Hi everyone, thanks for all your work and sorry for my tardiness on this. 

Reviewing Greg's patch, I'm fine with his approach. If you also submit a patch 
that 
does the following it will make it easier for me to apply:

* Update the Quick Start and User Manual to remove the old way of registering 
onload 
listeners and indicate how to do it this way.
* Create a simple test case in tests/browser-tests/issue-tests/issue375 that 
shows 
SVG Web and JQuery working together. Create a README.txt file that indicates 
the 
version of JQuery, it's license, and a bit of info on the test.
* Update the tests in tests/browser-tests/test_js1.html and test_js2.html to 
make 
sure they test the new functionality.
* Ensure this works with both embedded SVG right in the HTML and on an onload 
listener in an SVG file -- what happens for onload listeners inside SVG files? 
Does 
that stay the same?
* Create some simple "Porting Guide" text that I can put in the release notes 
on how 
this change affects anyone who used to use SVG Web since it is a breaking 
change.
* Update all the demos in samples/javascript-samples to use the new syntax.

These are all the things I have to do on my side which is why I've been a bit 
slow 
since my plate is full right now. If you submit a patch that does all these 
things I can 
apply it easier and then push out a new release sooner! Thanks everyone!

Original comment by bradneub...@gmail.com on 16 Feb 2010 at 6:18

GoogleCodeExporter commented 8 years ago
Hi everyone.

Sure Brad, I can do all the stuffs you need. Hopefully I'll have enough time to 
do
them within this week.

Original comment by gregorio...@gmail.com on 17 Feb 2010 at 8:28

GoogleCodeExporter commented 8 years ago
I added a test case for this issue under tests/browser-tests/issue-tests and 
checked
them in:

issue375-jquery-1.4.2.min.js
issue375.html
issue375.svg

Original comment by bradneub...@gmail.com on 6 Apr 2010 at 11:30

GoogleCodeExporter commented 8 years ago
This issue was closed by revision r1070.

Original comment by bradneub...@gmail.com on 7 Apr 2010 at 12:21

GoogleCodeExporter commented 8 years ago
Documentation on updating your code that uses SVG Web with this fix here:

http://codinginparadise.org/projects/svgweb/docs/UserManual.html#breaking_change
s

Original comment by bradneub...@gmail.com on 7 Apr 2010 at 5:40

GoogleCodeExporter commented 8 years ago
Issue 402 has been merged into this issue.

Original comment by bradneub...@gmail.com on 7 Apr 2010 at 6:38

GoogleCodeExporter commented 8 years ago
Issue 403 has been merged into this issue.

Original comment by bradneub...@gmail.com on 7 Apr 2010 at 6:53

GoogleCodeExporter commented 8 years ago
Issue 404 has been merged into this issue.

Original comment by bradneub...@gmail.com on 7 Apr 2010 at 8:33