Gisellameloni / svg-edit

Automatically exported from code.google.com/p/svg-edit
MIT License
0 stars 0 forks source link

Undo / redo breaks when using setSvgString (e.g. svg source editor) #1138

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Create any element
2. Go to the svg source editor and completely overwrite the current svg source 
with any other svg source
3. Click undo
4. Click on the original element you created

What is the expected output? What do you see instead?
You expect to be able to select the original element, but instead you get a 
"Cannot read property 'parentNode' of null" error.

Please use labels and text to provide additional information.
This seems to be related to the discussion that was started here: 
https://code.google.com/p/svg-edit/issues/detail?id=756

But unfortunately that discussion seems to have died out :( After lots of 
debugging though, I've got a pretty good handle on what's causing the error.

At a high level, the issue is that when the history BatchCommand "Change 
Source" (created in setSvgString) gets UN-applied, it successfully restores 
"svgroot" to the original content, so the canvas LOOKS okay, but it never 
updates "svgcontent" / "current_drawing_".

So after the undo, you click on an element that's in "svgroot" but not in the 
current drawing, and then "getMouseTarget" starts following "parentNode" 
references and comparing them to "current_layer" to figure out where to stop. 
Since "current_layer" is part of the current drawing (which is stale), it will 
never find the current layer and will keep following parents all the way up the 
DOM until it can't go up any higher and then fails.

My question is - why does setSvgString try to use the insert/remove element 
history commands - isn't it a special case in that it is changing the entire 
drawing? I think a special ChangeSourceCommand that keeps track of the old 
source vs. new source and calls setSvgString to apply them would work well 
here. I think the key is that when you're changing the source, you NEED to call 
setSvgString because it takes care of updating the current drawing.

What are the other project members' thoughts?

I'm happy to work on this if it is indeed a good solution.

Thanks!

Original issue reported on code.google.com by dan...@trydesignlab.com on 5 Sep 2013 at 7:29

GoogleCodeExporter commented 8 years ago
Drive-by comment... +1 for a ChangeSourceCommand

Original comment by codedr...@gmail.com on 5 Sep 2013 at 8:38