Bug 135698
Summary: | SVGElement has no innerHTML property | ||
---|---|---|---|
Product: | WebKit | Reporter: | Ophir LOJKINE <pere.jobs> |
Component: | SVG | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED DUPLICATE | ||
Severity: | Normal | CC: | caitp, damien.nozay, galineau, julien.sanchez, krit, zimmermann |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | All | ||
OS: | All |
Ophir LOJKINE
In webkit, we can't access the innerHTML of an inline <SVG> element in an html document.
Steps to reproduce:
In the console, type
'innerHTML' in SVGSVGElement.prototype
Results:
On Firefox and Chrome: true
On webkit: false
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Dirk Schulze
(In reply to comment #0)
> In webkit, we can't access the innerHTML of an inline <SVG> element in an html document.
>
> Steps to reproduce:
> In the console, type
>
> 'innerHTML' in SVGSVGElement.prototype
>
> Results:
> On Firefox and Chrome: true
Is that the case for any SVG element in an HTML document? Even for <rect>, <circle>? Or just outer <svg> element?
> On webkit: false
Ophir LOJKINE
This is the case for all SVG elements (they all inherit from SVGElement, which doesn't have innerHTML)
> 'innerHTML' in SVGRectElement.prototype
< false
> 'innerHTML' in SVGElement.prototype
< false
Dirk Schulze
(In reply to comment #2)
> This is the case for all SVG elements (they all inherit from SVGElement, which doesn't have innerHTML)
I was asking for Blink and Firefox. Does it work for all elements there, or just on the root <svg> element in HTML? (Beside all HTMLElements of course.)
>
> > 'innerHTML' in SVGRectElement.prototype
> < false
>
> > 'innerHTML' in SVGElement.prototype
> < false
Ophir LOJKINE
> I was asking for Blink and Firefox. Does it work for all elements there, or just on the root <svg> element in HTML? (Beside all HTMLElements of course.)
Yes, it works on every svg element on chrome and firefox (because all SVG elements inherit from SVGElement).
Dirk Schulze
(In reply to comment #4)
> > I was asking for Blink and Firefox. Does it work for all elements there, or just on the root <svg> element in HTML? (Beside all HTMLElements of course.)
>
> Yes, it works on every svg element on chrome and firefox
Ok. So they probably moved innerHTML on Element or added it to SVGElement. I assume innerHTML = "<rect>" still gives you an HTMLUnknownElement/HTMLElement on Blink and Firefox?
> (because all SVG elements inherit from SVGElement).
Hehe, SVG elements always inherit from SVGElement and always did. The problem is/was that innerHTML is just defined for HTMLElement.
Ophir LOJKINE
> Ok. So they probably moved innerHTML on Element or added it to SVGElement. I assume innerHTML = "<rect>" still gives you an HTMLUnknownElement/HTMLElement on Blink and Firefox?
Yes. In the Firefox console:
> div = document.createElement("div");
> div.innerHTML = "<svg></svg>";
> div.firstChild.innerHTML = "<rect>"
> div.firstChild.firstChild.__proto__
< HTMLUnknownElementPrototype
(But IMO, it's a pitty. It makes it more complex to use templates to create SVG elements, for example).
Ophir LOJKINE
And firefox folks intend to change that behavior:
https://bugzilla.mozilla.org/show_bug.cgi?id=886390
Ophir LOJKINE
And in chrome, it works:
< div = document.createElement("div");
< div.innerHTML = "<svg></svg>";
< div.firstChild.innerHTML = "<rect>"
< div.firstChild.firstChild.__proto__
> SVGRectElement
Caitlin Potter (:caitp)
The base interface for SVGElements does not have innerHTML exposed to scripts in its IDL --- in Blink and Gecko, innerHTML is exposed on the Element interface rather than HTMLElement interface, so they can share this common functionality.
It's probably not a terribly difficult patch to write to fix this, if I have some spare cycles in 2 weeks I'll be happy to take a shot at it.
Caitlin Potter (:caitp)
(In reply to comment #9)
> The base interface for SVGElements does not have innerHTML exposed to scripts in its IDL --- in Blink and Gecko, innerHTML is exposed on the Element interface rather than HTMLElement interface, so they can share this common functionality.
>
> It's probably not a terribly difficult patch to write to fix this, if I have some spare cycles in 2 weeks I'll be happy to take a shot at it.
fwiw, the webkit treatment appears to be spec-correct, but I would treat it as a bug due to being incompatible with 2 major vendors.
Julien Sanchez
(In reply to comment #10)
> fwiw, the webkit treatment appears to be spec-correct, but I would treat it as a bug due to being incompatible with 2 major vendors.
It's no more correct. The spec has been fixed last year [1]. innerHTML is now defined on Element (implemented in Chrome [2] and Firefox) *and* should create elements with correct namespace (ok in chrome [2], not yet in Firefox [3]).
[1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=17924
[2] https://code.google.com/p/chromium/issues/detail?id=311080
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=886390
Julien Sanchez
Testcase (http://marianoguerra.github.io/bugs/svgbug/) from the following issue on Github: https://github.com/facebook/react/issues/1900 .
Webkit is also impacted as innerHTML is not defined on SVG elements.
Only the second example ("innerHTML") is relevant here.
It is certainly more complicated than just define innerHTML on Element because it should also create the <text> node as SVGTextElement and not as HTMLUnknownElementPrototype.
Caitlin Potter (:caitp)
(In reply to comment #11)
> (In reply to comment #10)
> > fwiw, the webkit treatment appears to be spec-correct, but I would treat it as a bug due to being incompatible with 2 major vendors.
>
> It's no more correct. The spec has been fixed last year [1]. innerHTML is now defined on Element (implemented in Chrome [2] and Firefox) *and* should create elements with correct namespace (ok in chrome [2], not yet in Firefox [3]).
>
> [1] https://www.w3.org/Bugs/Public/show_bug.cgi?id=17924
> [2] https://code.google.com/p/chromium/issues/detail?id=311080
> [3] https://bugzilla.mozilla.org/show_bug.cgi?id=886390
Not according to any of the DOM specs which have made it to REC, or the WHATWG's DOM.
However, this is academic, because if 2 major vendors are exposing this on Element, then it really doesn't matter what the spec says.
(In reply to comment #12)
> Testcase (http://marianoguerra.github.io/bugs/svgbug/) from the following issue on Github: https://github.com/facebook/react/issues/1900 .
> Webkit is also impacted as innerHTML is not defined on SVG elements.
>
> Only the second example ("innerHTML") is relevant here.
> It is certainly more complicated than just define innerHTML on Element because it should also create the <text> node as SVGTextElement and not as HTMLUnknownElementPrototype.
Yes, it's obviously necessary to use the correct parsing insertion mode. By "not terribly difficult" I'm not saying it's a one-line patch, just that it's clear what is needed to be done.
Would you care to write the patch? I will not have time for at least 2 weeks.
Julien Sanchez
> Not according to any of the DOM specs which have made it to REC, or the WHATWG's DOM.
>
> However, this is academic, because if 2 major vendors are exposing this on Element, then it really doesn't matter what the spec says.
Right. I was referring to the DOM parsing living standard and latest candidate rec.
> Yes, it's obviously necessary to use the correct parsing insertion mode. By "not terribly difficult" I'm not saying it's a one-line patch, just that it's clear what is needed to be done.
>
> Would you care to write the patch? I will not have time for at least 2 weeks.
I would be really happy to do it. Unfortunately, I am afraid that it would be too difficult for me and presently I have neither enough time nor deep technical knowledge of Webkit to make it right and quick (within your two weeks for instance). However, I will try have a look at least out of curiosity.
Caitlin Potter (:caitp)
(In reply to comment #14)
> > Not according to any of the DOM specs which have made it to REC, or the WHATWG's DOM.
> >
> > However, this is academic, because if 2 major vendors are exposing this on Element, then it really doesn't matter what the spec says.
>
> Right. I was referring to the DOM parsing living standard and latest candidate rec.
>
> > Yes, it's obviously necessary to use the correct parsing insertion mode. By "not terribly difficult" I'm not saying it's a one-line patch, just that it's clear what is needed to be done.
> >
> > Would you care to write the patch? I will not have time for at least 2 weeks.
>
> I would be really happy to do it. Unfortunately, I am afraid that it would be too difficult for me and presently I have neither enough time nor deep technical knowledge of Webkit to make it right and quick (within your two weeks for instance). However, I will try have a look at least out of curiosity.
Cool, the Blink implementation should serve as somewhat of a guide --- it looks like much of editing/markup's API has not changed too much since the Blink fork, and the Blink innerHTML routine is just a call to createMarkup() with "this" (*this in the case of WebKit) and the element's childNodes --- It probably needs some adjustment to work correctly in WebKit, but following those lines is probably adequate, provided WebKit's createMarkup() is smart enough to figure out the correct insertion mode to use.
Julien Sanchez
(In reply to comment #15)
> Cool, the Blink implementation should serve as somewhat of a guide --- it looks like much of editing/markup's API has not changed too much since the Blink fork, and the Blink innerHTML routine is just a call to createMarkup() with "this" (*this in the case of WebKit) and the element's childNodes --- It probably needs some adjustment to work correctly in WebKit, but following those lines is probably adequate, provided WebKit's createMarkup() is smart enough to figure out the correct insertion mode to use.
Thanks a lot for the guidelines. I didn't even think to look at related Blink changes. Now that I did, it doesn't seem so hard to try and adapt them.
Damien Nozay [:dnozay]
see also https://bugs.webkit.org/show_bug.cgi?id=136903 (bug 136903)
there is some work in progress there.
Sylvain Galineau
*** This bug has been marked as a duplicate of bug 136903 ***