rebootcode / svg-edit

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

Chrome 15: Not possible to import/open anything #889

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
**What steps will reproduce the problem?**
1. Open Chrome 15 (on Firefox this works normal) on page 
http://svg-edit.googlecode.com/svn/trunk/editor/svg-editor.html
2. click import or open
3. import/open any SVG file

**What is the expected output? What do you see instead?**
Open/import of the file.

**In what browser did you experience this problem? (ALL, Firefox, Opera, etc)**
Tried on Chrome 15 - fails with DomException
                     code: 9
                     message: "NOT_SUPPORTED_ERR: DOM Exception 9"
                     name: "NOT_SUPPORTED_ERR"
    in svgcanvas.js on line 5989

Firefox 8 - works ok

**In what version of SVG-edit does the problem occur? (Latest trunk, 2.5.1, 
etc)**
Tried 2.5.1 and trunk. Behaviour is same.

Original issue reported on code.google.com by honzapr...@gmail.com on 15 Dec 2011 at 7:26

GoogleCodeExporter commented 9 years ago
i think there a problem with your svg file. i believe chrome does not support 
javascript inside svg file.

try to download this one
http://upload.wikimedia.org/wikipedia/commons/7/70/US-FBI-ShadedSeal.svg

and import it in. it's working on my chrome version 15

Original comment by PaulS...@gmail.com on 15 Dec 2011 at 10:02

GoogleCodeExporter commented 9 years ago
You file works for me. I tried my file, generated from Corel Draw. There is no 
JavaScript inside. File is attached.

After some trying, it seems that <defs> section cause the problems.

Original comment by honzapr...@gmail.com on 2 Jan 2012 at 5:08

Attachments:

GoogleCodeExporter commented 9 years ago
@PaulSoft Chrome can load script-enabled SVGs. My project deal exclusively with 
interactive SVGs with script and style tags.

@honzaprace that error is generally defined as
    NOT_SUPPORTED_ERR: If the implementation does not support the requested type of object or operation.

Or specifically to importNode() as
    NOT_SUPPORTED_ERR: Raised if the type of node being imported is not supported.

Why?  Because <style> is not in whitelist by default! So your style tag will be 
stripped leaving only the orphaned CSS... :(

So I added style to whitelist but I still get that error. It turns out the root 
problem is your CDATA tag. Without that everything then works just fine.

I am unsure why or where the CDATA handling is done -- no time to trace that 
yet. But at least we now know the exact reason.

A fix for your <style> is to add this line in editor/sanitize.js:55
"defs" : [],
"style" : ["type"], //Add this to allow style element and type attribute for it
"desc" : [],

Original comment by asyazwan on 23 Feb 2012 at 10:04

GoogleCodeExporter commented 9 years ago
Hi again,

Look what I found: http://code.google.com/p/chromium/issues/detail?id=57871

It's a known Chromium issue!

Easy to solve this then. If we detect Chrome then we do manual cloning 
(leveraging jQuery.clone()).

Here are my patches. I'm unsure about the sanitize.js if users have to always 
manually add their potentially dangerous tags. Any devs can chime in?

Original comment by asyazwan on 23 Feb 2012 at 11:08

Attachments:

GoogleCodeExporter commented 9 years ago
My worry with adding style is that there might be a possibility of including 
maliciousness in some css property (like sourcing an external SVG file that 
contains script or something).  I have no idea if it's actually possible, was 
just taking a cautious stance back then.  I don't feel too strongly about it 
these days though.

Original comment by codedr...@gmail.com on 23 Feb 2012 at 8:09

GoogleCodeExporter commented 9 years ago
Ahmad, I merged in your patches with r2057.  Please close this bug if you think 
it's fixed now and thanks again! :)

Original comment by codedr...@gmail.com on 26 Feb 2012 at 6:09

GoogleCodeExporter commented 9 years ago
Unfortunately the patch only fixes Open image. Import image uses another 
function which is unpatched. Even if patched, importing doesn't really work. 
I'll need to study the import mechanism as it's not as straight-forward as open 
image. (e.g. with sample above import creates <symbol> elements and put 
everything in there and then uses <use> in layer to refer to imported stuff)

So I'll leave this open until then.

Original comment by asyazwan on 27 Feb 2012 at 4:54

GoogleCodeExporter commented 9 years ago
Hi, please apply this patch instead which I think is the best solution.

The previous patch I skipped importNode() completely. Which according to the 
spec, is wrong. Read more if you're interested: 
https://code.google.com/p/svg-edit/source/detail?r=2057&url_prefix=p

Also in this patch I am making use of adoptNode if supported. adoptNode is not 
buggy in Chrome making the whole isChrome() thing obsolete. adoptNode is also 
semantically what we want in this particular use case, not importNode.

In case adoptNode is not available (it is as far as FF & Chrome is concerned), 
we fall back to the good ol' importNode().

Open Image should now be solved.

I'll look into Import Image later.

Original comment by asyazwan on 27 Feb 2012 at 10:31

Attachments:

GoogleCodeExporter commented 9 years ago
Hi again,

I've fixed everything, this patch includes the above patch since it has not 
been applied yet.

In summary:
- Add CDATA support when View/Edit source
- Apply Open Image fix to Import Image as well (adoptNode)

That's it.

@honzaprace: If you import your sample (Bez názvu - 1.svg) it will be scaled 
to extremely small size (required me to zoom in 6000%+). The import code 
re-scale everything with the formula (canvasmax / 3) / importedmax. So in your 
case and default SE dimension it will be (480 / 3) / 29700 = 
0.0053872053872054. Why is this the case? I have no idea. :)
So it's best to import SVGs with their content's dimension close to the its 
document dimension (in your case the rect is 5389x6912 but the svg doc is 
21000x29700!!).

Original comment by asyazwan on 27 Feb 2012 at 12:34

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Oops, tab/space issue

Original comment by asyazwan on 27 Feb 2012 at 12:38

Attachments:

GoogleCodeExporter commented 9 years ago
r2059 committed.  Thanks again asyazwan!  Btw, next patch can you just name it 
after the bug number of something?  This will make my management of patches a 
little easier.  Please close this bug once you've verified.

Original comment by codedr...@gmail.com on 27 Feb 2012 at 5:24

GoogleCodeExporter commented 9 years ago
Roger on the naming scheme.

Verified to work on latest Chrome & FF using sample by issuer and some other of 
my own.

Original comment by asyazwan on 27 Feb 2012 at 6:18