Closed
Bug 660660
Opened 14 years ago
Closed 4 years ago
Remove CDATASection
Categories
(Core :: DOM: Core & HTML, defect, P5)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
INVALID
People
(Reporter: Ms2ger, Unassigned)
References
()
Details
Attachments
(6 files, 4 obsolete files)
6.97 KB,
patch
|
sicking
:
review-
|
Details | Diff | Splinter Review |
3.14 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
1.29 KB,
patch
|
sicking
:
review-
|
Details | Diff | Splinter Review |
1.75 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
18.37 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
12.82 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•14 years ago
|
||
Attachment #536102 -
Flags: review?(jonas)
Reporter | ||
Comment 2•14 years ago
|
||
Still needs uuid updates
Attachment #536103 -
Flags: review?(jonas)
Reporter | ||
Comment 3•14 years ago
|
||
Attachment #536104 -
Flags: review?(jonas)
Reporter | ||
Comment 4•14 years ago
|
||
Attachment #536105 -
Flags: review?(jonas)
![]() |
||
Comment 5•14 years ago
|
||
Do we actually want this change? What do other UAs do? Doesn't this break serialization roundtripping?
Hmm.. don't we want the serializer to use <![CDATA[]]> for things that were cdata sections in the original source?
Basically I have the same concern as comment 5.
Could we keep an internal state flag on the normal nsTextNode which remembers if it was a cdata section or not and have the serializer pay attention to that?
Please do not do that ! BlueGriffon needs CDATA section to create <style>
and <script> elements containing CDATA sections from DOM calls and we need
the CDATA sections created that way to remain CDATA in the DOM; if you turn
them into text nodes, it will be a real pain to deal with these elements in
an editing environment.
What I think we should do is to add a property named 'mozSerializeAsCDATA' on nsIDOMText. The property should be a read/write boolean.
Then make the XML parser set this property after creating the textnode when parsing a CDATA section.
Likewise, the serializer should use this property when serializing.
We'd probably back the property with a nsINode flag. I'm sure there are plenty of flags that only apply to elements, so we can reuse one of those bits.
However I *do* strongly think we should make .nodeType return the same value as for normal text nodes. And I *do* think that we should nuke document.createCDATASection.
This way the platform provides the same functionality as we have now, but we use a better API for it which doesn't force everyone to keep CDATA sections in mind.
Attachment #536103 -
Flags: review?(jonas) → review+
Hmm... It is a tricky question how to parse markup like:
<foo>text here <![CDATA[and here]]></foo>
<foo> <![CDATA[goop]]> </foo>
I definitely think that in both cases we want to create a single textnode inside the <foo> element. The question is if the 'mozSerializeAsCDATA' flag should be set or not.
The former case is pretty rare I suspect, and not something that I can recall ever seeing in a real-world document. I suspect it'd be fine to create a single "non-cdata-serializing" node.
For the latter case it might be worth setting the cdata flag.
Jonas, please please no! Whatever you are planning with a future DOM spec,
that spec is not a standard yet. For the time being, the only standard
says how a UA should behave in front of a CDATA and nodeType should
reflect it. Firefox is not the only app based on the Mozilla platform
and you are going to impact them with this change.
If *internally* a CDATA section and a text node are just the same beast
with a flag, that's fine. As soon as parsing, DOM calls and serialization
are unchanged, I can live with that. If you change that, then I disagree
with the change because of its potential impact on the mozilla ecosystem.
In particular, I *do need* document.createCDATASection. Do NOT nuke it.
- editing environments
- tools needing cdata sections in proprietary xml-based or xhtml-based
exchange formats
- ...
With my proposal all you'd do is change
x = document.createCDATASection('foo');
to
x = document.createTextNode('foo');
x.mozSerializeAsCDATA = true;
So no functionality would go away.
Comment 12•14 years ago
|
||
Adding something like mozSerializeAsCDATA looks pretty hacky.
I'd rather just keep CDATASection.
And (In reply to comment #11)
> With my proposal all you'd do is change
>
> x = document.createCDATASection('foo');
>
> to
>
> x = document.createTextNode('foo');
> x.mozSerializeAsCDATA = true;
>
> So no functionality would go away.
Urgh.
Cool. Going to break all webapps based on createCDATASection ! Probably not
a lot but still. This is breaking DOM Core conformance and I think this change
is plain crazy.
A WAY BETTER option is to keep document.createCDATASection and make it
internally do what you want as soon as
- in the DOM, CDATA nodes still have the correct Node.type;
put whatever is needed based on your hacky mozSerializeAsCDATA to reply
the correct 'type'
- serialization is unchanged
I just cannot understand this proposed change. Does not make sense at all
to break conformance to DOM Core.
(In reply to comment #12)
> Adding something like mozSerializeAsCDATA looks pretty hacky.
> I'd rather just keep CDATASection.
It's a pain in the ass to ask everyone to change their x.nodeType == TEXT_NODE to
x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE.
The end result is simply that people will avoid using CDATA in their markup which isn't a good solution either and will just result in bugs as people do escaping wrong.
(In reply to comment #14)
> (In reply to comment #12)
> > Adding something like mozSerializeAsCDATA looks pretty hacky.
> > I'd rather just keep CDATASection.
>
> It's a pain in the ass to ask everyone to change their x.nodeType ==
> TEXT_NODE to
> x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE.
Oh come on. That's the worst excuse ever for a change related to
standards conformance.
> The end result is simply that people will avoid using CDATA in their markup
> which isn't a good solution either and will just result in bugs as people do
> escaping wrong.
But CDATA are NOT simple Text nodes. They have implications on
delimiters like < and allowing a prefixedread-write attribute
on nodes to tweak that behavior and the serialization is just
a ugly hack in the name of footprint optimization.
I still have no idea why you're proposing this change. Care to explain?
Is it a WHATWG thing trying to get rid of CDATA in <style> and <script>
in a new "cleanup of the past" effort? I suspect it is.
Comment 16•14 years ago
|
||
(In reply to comment #12)
> Adding something like mozSerializeAsCDATA looks pretty hacky.
> I'd rather just keep CDATASection.
I agree. If we are going so support serializing CDATA sections, I'd rather keep around the old node type than to add a new flag.
(In reply to comment #14)
> It's a pain in the ass to ask everyone to change their x.nodeType ==
> TEXT_NODE to
> x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE.
If that's a strong concern, we could still make the XML parser never emit CDATA section nodes in Firefox (and let BlueGriffon flip a pref or define to emit CDATA sections) so that the only way to get one in Firefox would be calling document.createCDATASection.
(In reply to comment #15)
> But CDATA are NOT simple Text nodes.
Semantically they are. Letting the syntactic sugar leak to the data model has always been a domain modeling error in the design of the DOM.
> I still have no idea why you're proposing this change. Care to explain?
> Is it a WHATWG thing trying to get rid of CDATA in <style> and <script>
> in a new "cleanup of the past" effort? I suspect it is.
I don't think the WHATWG is trying anything in particular relative to the XML DOM. The same people might be trying stuff in Web Apps, though. However, for the HTML DOM, the WHATWG is trying to avoid introducing the CDATA cruft, so the HTML5 parsing algorithm never emits CDATA nodes even though <![CDATA[...]]> is supported in the syntax in SVG and MathML contexts.
(In reply to comment #16)
> I don't think the WHATWG is trying anything in particular relative to the
> XML DOM. The same people might be trying stuff in Web Apps, though. However,
> for the HTML DOM, the WHATWG is trying to avoid introducing the CDATA cruft,
> so the HTML5 parsing algorithm never emits CDATA nodes even though
> <![CDATA[...]]> is supported in the syntax in SVG and MathML contexts.
So you're saying that even in the xml serialization of html5, a cdata section
present in the markup will not generate a cdata section node ?!? Wow, to say
the least.
(In reply to comment #16)
> (In reply to comment #15)
> > But CDATA are NOT simple Text nodes.
>
> Semantically they are. Letting the syntactic sugar leak to the data model
> has always been a domain modeling error in the design of the DOM.
Who cares about semantics today when something has been deployed all over
the place for 15 years and is part of XML ?!?
This just not a valid argument enough. If you push your argument further
then HTML5 should immediately get rid of the <pre> element.
I vote for a WONTFIX for this bug.
Comment 19•14 years ago
|
||
(In reply to comment #17)
> (In reply to comment #16)
>
> > I don't think the WHATWG is trying anything in particular relative to the
> > XML DOM. The same people might be trying stuff in Web Apps, though. However,
> > for the HTML DOM, the WHATWG is trying to avoid introducing the CDATA cruft,
> > so the HTML5 parsing algorithm never emits CDATA nodes even though
> > <![CDATA[...]]> is supported in the syntax in SVG and MathML contexts.
>
> So you're saying that even in the xml serialization of html5, a cdata section
> present in the markup will not generate a cdata section node ?!?
I pretty carefully did *not* say that.
Comment 20•14 years ago
|
||
I am the author of an XML wysiwyg editor based on gecko, and I can say the remove of the support of CDATA sections is a very bad idea. And I don't see the advantage to replace it by a mozSerializeAsCDATA property.
Comment 21•14 years ago
|
||
(In reply to comment #15)
> CDATA in <style> and <script>
BTW, why does BlueGriffon care about CDATA nodes in the DOM instead of having a serializer that always serializes the text content of <script> and <style> as CDATA sections in XHTML output? After all, a DOM-based editor can't round-trip syntactic sugar anyway in the general case, since you've already lost information about which characters were escaped as named character references, decimal character references or hexadecimal character references and information about what kind of whitespace was between attributes.
(In reply to comment #21)
> (In reply to comment #15)
> > CDATA in <style> and <script>
>
> BTW, why does BlueGriffon care about CDATA nodes in the DOM instead of
> having a serializer that always serializes the text content of <script> and
> <style> as CDATA sections in XHTML output? After all, a DOM-based editor
> can't round-trip syntactic sugar anyway in the general case, since you've
> already lost information about which characters were escaped as named
> character references, decimal character references or hexadecimal character
> references and information about what kind of whitespace was between
> attributes.
Because Laurent Jouanneau invested a lot of time in the HTML serializer to
make it better and BlueGriffon relies on it? Because writing our own
serializer is a rather big task and a footprint hit since Gecko already has
a good one? Because I never thought Mozilla engineers supposed to care about
existing and deployed Web standards could one day try to remove something
that is still a standard and has been living peacefully in our implementations
since november 1998?
Comment 23•14 years ago
|
||
(In reply to comment #22)
> Because Laurent Jouanneau invested a lot of time in the HTML serializer to
> make it better and BlueGriffon relies on it? Because writing our own
> serializer is a rather big task and a footprint hit since Gecko already has
> a good one?
If Gecko provided an X(HT)ML serializer that was configurable to always serialize the content of <script> and <style> elements as CDATA sections, would you still have use cases for CDATA sections in the DOM?
(In reply to comment #23)
> If Gecko provided an X(HT)ML serializer that was configurable to always
> serialize the content of <script> and <style> elements as CDATA sections,
> would you still have use cases for CDATA sections in the DOM?
1. yes. If the user does not edit his stylesheets and scripts, I must
output the elements as they are in the document I have loaded. No CDATA
in, no CDATA out ; CDATA in, CDATA out. Your "always" is bad for BG
or other apps like Etna.
2. I am still opposed to the proposed change anyway, since it decreases
standards conformance of the platform
Comment 25•14 years ago
|
||
I'd like to know where has the discussion about removing CDATASection happening?
I couldn't find any emails in WebApps WG, and that is where Web DOM Core
is being written.
Comment 26•14 years ago
|
||
(In reply to comment #23)
> If Gecko provided an X(HT)ML serializer that was configurable to always
> serialize the content of <script> and <style> elements as CDATA sections,
> would you still have use cases for CDATA sections in the DOM?
Be careful here. You try to reinvent the while. You propose some workaround to deal with a special use case when the current serializer already deal with it in a general way. In fact there is a lot of other use cases where CDATA sections can be relevant when you have to deal with a true XHTML document (and it's worst with XML documents). As Daniel point it out, <pre> is also a concern.
As a frond end developer, I don't care the way Gecko handle CDATA internally but I really need to have a browser that is as standard compliant as possible. At that point, the DOM spec said how a CDATA section is exposed in the DOM. It would be very disruptive to break the DOM API intentionally and just painful for developers. Web standards are not just here to have fun and breaking them has consequences.
(In reply to comment #23)
> It's a pain in the ass to ask everyone to change their x.nodeType == TEXT_NODE to
> x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE.
Yes maybe but it's the standard way, which mean that it's the way that should be preferred to deal with a cross browser application (and frankly, it's not that painful with any basic search/replace feature of any basic text editor).
It's absolutely acceptable to have a new and simpler API to deal with the CDATA sections, BUT this does not mean to remove the old one which is both standard and cross browser compliant.
If some people at Mozilla are disturb by the way the DOM API deal with CDATA, you should consider disusing it with the relevant W3C group first, just to see if your proposal has some echo with other browser vendor intention. If it's not the case, I beg you to throw this idea away instead of breaking things that are working nowadays.
![]() |
||
Comment 27•14 years ago
|
||
Jeremie, just to be clear the relevant W3C group's current DOM Core draft doesn't have CDATA section nodes in it. So either we should remove them or they need to add them.
(In reply to comment #27)
> Jeremie, just to be clear the relevant W3C group's current DOM Core draft
> doesn't have CDATA section nodes in it. So either we should remove them or
> they need to add them.
The latter. DOM Core is not only for HTML. XML has CDATA sections and the DOM
needs to support them.
Comment 29•14 years ago
|
||
(In reply to comment #24)
> 1. yes. If the user does not edit his stylesheets and scripts, I must
> output the elements as they are in the document I have loaded.
How's that different from roundtripping entities or the pretty-printing between attributes or choice of attribute quotes, etc.?
For example:
<div
class=foo
id='bar'
>äää</div>
That stuff isn't preserved, either, and has exactly as much significance as the distinction between a CDATA section and normal text content.
(In reply to comment #29)
> How's that different from roundtripping entities or the pretty-printing
> between attributes or choice of attribute quotes, etc.?
>
> For example:
> <div
> class=foo
> id='bar'
> >äää</div>
>
> That stuff isn't preserved, either, and has exactly as much significance as
> the distinction between a CDATA section and normal text content.
They're all major feedbacks of Nvu/KompoZer/BlueGriffon users. The fact
the DOM implemented in Gecko is clearly browser-oriented with simplifications
needed by footprint and perf can be a burden on other apps, absolutely.
No need to add to that list with something that belongs to XML anyway,
and then needs to be exposed through DOM Core anyway.
(In reply to comment #24)
> (In reply to comment #23)
>
> > If Gecko provided an X(HT)ML serializer that was configurable to always
> > serialize the content of <script> and <style> elements as CDATA sections,
> > would you still have use cases for CDATA sections in the DOM?
>
> 1. yes. If the user does not edit his stylesheets and scripts, I must
> output the elements as they are in the document I have loaded. No CDATA
> in, no CDATA out ; CDATA in, CDATA out. Your "always" is bad for BG
> or other apps like Etna.
With my proposed changes that's exactly what would happen. I.e. we would retain round-trippability.
> 2. I am still opposed to the proposed change anyway, since it decreases
> standards conformance of the platform
Actually, this change is to *increase* standards conformance as the latest version of DOM-Core removes CDATA section support.
I definitely agree we shouldn't do this unless the spec says so.
On the flip side, I also don't want to introduce the complexity of having two types of text nodes to millions of webapps, just to make it easier for a handful of editor apps. Especially when we can still satisfy the editor apps use cases using other API.
(In reply to comment #16)
> (In reply to comment #12)
> > Adding something like mozSerializeAsCDATA looks pretty hacky.
> > I'd rather just keep CDATASection.
>
> I agree. If we are going so support serializing CDATA sections, I'd rather
> keep around the old node type than to add a new flag.
Why?
(In reply to comment #31)
> (In reply to comment #24)
> > (In reply to comment #23)
> >
> > > If Gecko provided an X(HT)ML serializer that was configurable to always
> > > serialize the content of <script> and <style> elements as CDATA sections,
> > > would you still have use cases for CDATA sections in the DOM?
> >
> > 1. yes. If the user does not edit his stylesheets and scripts, I must
> > output the elements as they are in the document I have loaded. No CDATA
> > in, no CDATA out ; CDATA in, CDATA out. Your "always" is bad for BG
> > or other apps like Etna.
>
> With my proposed changes that's exactly what would happen. I.e. we would
> retain round-trippability
No. The proposed change gets rid of document.createCDATASection(). That's bad
and, again, CDATA belongs to XML so it's very likely DOM Core next level will
have to include createCDATASection(), at least for compatibility reasons
with the existing DOM-based apps, browser-based or not.
Comment 33•14 years ago
|
||
I think this is a Bad Idea too.
Reason: Right now, if I want to cut & paste a script or stylesheet from an external file to an inline element (or vice versa), a CDATA section means I can do just that without having to worry about encoding entities like < or >. Without CDATA sections, it's extra work, potentially a lot in the case of an inline script.
Also, it seems strange to remove a method just because the specification removes it. Typically, a specification states the *minimum* that we implement... it's perfectly okay for us to have extras on top of the specification, as long as we implement the spec. So CDATA sections, and methods to create them, simply become extras. That doesn't imply we should remove them.
(In reply to comment #27)
> Jeremie, just to be clear the relevant W3C group's current DOM Core draft
> doesn't have CDATA section nodes in it. So either we should remove them or
> they need to add them.
It sounds like this is where the discussion should be taking place, not in this bug. All of this has appeared in Bugzilla in the last 3 days... we should slow down a bit, and take our arguments to the W3C mailing lists first.
This sentence from the spec (listed in the URL field, section 9.2) worries me a lot:
"The remainder of interfaces and interface members listed in this section were once part of DOM Core. Implementations conforming to this specification will not support them."
Sure enough, CDATASection is listed there. We're barking up the wrong tree, and ironically there's a W3C bug filed (by this bug's reporter, no less!) to restore CDATASection to the spec:
http://www.w3.org/Bugs/Public/show_bug.cgi?id=12841
![]() |
||
Comment 34•14 years ago
|
||
> if I want to cut & paste a script or stylesheet from an external file
This will continue to work. Again, this bug is about the DOM, not about the syntactic sugar. The latter is clearly required for anything resembling web compat.
> it's perfectly okay for us to have extras on top of the specification
In the case of DOM APIs, only if they're prefixed with "moz".
Comment 35•14 years ago
|
||
> (In reply to comment #16)
> > (In reply to comment #12)
> > > Adding something like mozSerializeAsCDATA looks pretty hacky.
> > > I'd rather just keep CDATASection.
> >
> > I agree. If we are going so support serializing CDATA sections, I'd rather
> > keep around the old node type than to add a new flag.
>
> Why?
My thinking was that if we are going to have a way to create nodes that serialize as CDATA sections (as opposed to letting XULRunner apps ask that script and style content be serialized as CDATA sections in XML output), we might as well use the existing document.createCDATASection() API for creating them and at that point, it might as well return CDATA section nodes if the script writer knowingly called that API.
However, thinking about this more, the main expected win is getting rid of the x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE pattern, so if we remove anything here, we should probably make it so that no node in the DOM ever returns CDATA_SECTION_NODE from .nodeType no matter how created.
But if we then have a flag on text nodes to request nodes that serialize as CDATA sections, it would seem more backwards-compatible to have document.createCDATASection() return a text node with the flag set than to remove document.createCDATASection() and make all scripts that currently use it fail.
Note that having a flag is not without problems: How many child nodes would the parser create for element foo in this case: <foo>a<![[CDATA[[b]]>c</foo>? How would .normalize() behave if one of the adjacent nodes has the flag set.
To be clear, I think CDATA nodes in the DOM should never have existed in the first place, so if there's a way to get rid of them without breaking the Web, I think it's worth pursuing.
> However, thinking about this more, the main expected win is getting rid of
> the x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE pattern, so
> if we remove anything here, we should probably make it so that no node in
> the DOM ever returns CDATA_SECTION_NODE from .nodeType no matter how created.
Indeed. This is my main goal.
> But if we then have a flag on text nodes to request nodes that serialize as
> CDATA sections, it would seem more backwards-compatible to have
> document.createCDATASection() return a text node with the flag set than to
> remove document.createCDATASection() and make all scripts that currently use
> it fail.
How much web breakage can we really expect by removing createCDATASection? My guess would be very little. Google code search finds a few hits, though most of them seem to be various shims and such *implementing* a DOM rather than using it.
So we should weigh the benefit of keeping those callers working against the added confusion of having an API which sounds like it creates a "real" CDATA section node, but doesn't.
But if keeping createCDATASection makes it possible to fix this bug, or makes it significantly easier to do so, then I'm fine with that. Again, my main gripe is the fact that .nodeType is != TEXT_NODE.
> Note that having a flag is not without problems: How many child nodes would
> the parser create for element foo in this case:
> <foo>a<![[CDATA[[b]]>c</foo>?
Indeed. See comment 9 for some of my thoughts.
> How would .normalize() behave if one of the
> adjacent nodes has the flag set.
I absolutely think that we should merge all adjacent cdata and textnodes into a single node. Weather the resulting node should have the 'serialize as cdata' bit set or not is less important to me. One simple solution would be to just merge all nodes into the first one and use whatever bit it has.
> To be clear, I think CDATA nodes in the DOM should never have existed in the
> first place, so if there's a way to get rid of them without breaking the
> Web, I think it's worth pursuing.
My feelings exactly.
Possibly one solution is to keep createCDATASection for now but make it warn in the console whenever used, but remove the separate node implementation.
Comment 38•14 years ago
|
||
This seems pretty invasive just to avoid typing out "x.nodeType == TEXT_NODE || x.nodeType == CDATA_SECTION_NODE" every so often. Just write a macro or a function you can call instead, or make your editor do it for you.
![]() |
||
Comment 39•14 years ago
|
||
Daniel, you're saying that this is more invasive than forcing every single web developer remember to check for nodeType == CDATA_SECTION_NODE? I don't buy that.
(In reply to comment #39)
> Daniel, you're saying that this is more invasive than forcing every single
> web developer remember to check for nodeType == CDATA_SECTION_NODE? I don't
> buy that.
Boris, somme comments above or in the bmo bug say CDATA is unused on the web
(but it is in the XML world). Here you're saying that developers have to
remember to check nodeType for Node.CDATA_SECTION_NODE...
I agree with Daniel Brooks. Removing support for CDATA just in the name of
|nodeType == CDATA_SECTION_NODE| is a bad excuse for something breaking
compatibility with batch xml processors, xml editors, legacy xhtml tools.
CDATA are not perfect, are not widely used on the web, but they exist.
I still don't understand why you want to remove them and break 13 years
of backwards compatibility.
![]() |
||
Comment 41•14 years ago
|
||
CDATA is unused on the web to a first approximation.
Anyone writing script that will run against markup they don't completely control has to deal with the fact that someone might stick CDATA in there.
It's the worst of both worlds. Either CDATA being widely used or it not being used at all would be better than what we have now.
Here is what I think we should do:
1. Remove the CDATA Section node implementation
2. Add a .mozSerializeAsCDATA property on nsIDOMText
3. Make createCDATASection create a textnode but and set its mozSerializeAsCDATA
property to true.
4. Make the XMLContentSink merge cdatasection text data with preceding text data
just like it does for normal text data. However make it also set the
mozSerializeAsCDATA property to true when creating the textnode if any of
the data came from a CDATA section in the markup.
5. Make the serializer honor the mozSerializeAsCDATA property
6. Add a warning whenever createCDATASection is called stating that it's
deprecated and will be removed. (We'll probably remove it for FF8 or some
such).
7. File a bug to make the HTML parser set the mozSeralizeAsCDATA property
similarly to how the XMLContentSink does.
I'll also say that I'd accept patches to make the XMLContentSink not merge CDATA sections with surrounding text nodes if a hidden pref is set. To allow editors like BlueGriffon to retain roundtripability.
However I suspect that implementing that will get somewhat more complex down the road as we should make CDATA parsing be streaming. The fact that it currently isn't puts XML parsing at a severe disadvantage from HTML. This means that if you want to retain one-cdata-section-node-per-cdata-section-in-markup you have to come up with an API that does that while still allow streaming of CDATA sections.
Comment 43•14 years ago
|
||
(In reply to comment #42)
> 2. Add a .mozSerializeAsCDATA property on nsIDOMText
This is so ugly hack that I strongly object it.
(In reply to comment #43)
> (In reply to comment #42)
> > 2. Add a .mozSerializeAsCDATA property on nsIDOMText
> This is so ugly hack that I strongly object it.
I'm all ears for alternatives. But changing the .nodeType just to affect how it should be serialized is IMO much worse.
Comment 45•14 years ago
|
||
(In reply to comment #42)
> 7. File a bug to make the HTML parser set the mozSeralizeAsCDATA property
> similarly to how the XMLContentSink does.
That seems like introducing *more* CDATA section handling than what exists now. Can we at least defer doing that until we have solid evidence that not doing it is a real problem for someone?
That works for me.
Comment on attachment 536102 [details] [diff] [review]
Part a: Make the XML parser create Text nodes
Removing requests here until there are updated patches.
Attachment #536102 -
Flags: review?(jonas)
Attachment #536104 -
Flags: review?(jonas)
Attachment #536105 -
Flags: review?(jonas)
Also, the more I think about it, the more I like the mozSerializeAsCDATA idea.
The only difference between text nodes and cdata nodes is how they are serialized. They otherwise are semantically exactly the same thing. So it seem to me that using the same node type for the two, with a property that describes how they should be serialized is the correct solution. Using .nodeType to describe how to serialize is what feels like a hack to me.
Reporter | ||
Comment 49•14 years ago
|
||
Attachment #545233 -
Flags: review?(jonas)
Reporter | ||
Comment 50•14 years ago
|
||
Attachment #545234 -
Flags: review?(jonas)
Reporter | ||
Comment 51•14 years ago
|
||
Attachment #545235 -
Flags: review?(jonas)
Reporter | ||
Comment 52•14 years ago
|
||
Attachment #545236 -
Flags: review?(jonas)
Reporter | ||
Comment 53•14 years ago
|
||
Attachment #545237 -
Flags: review?(jonas)
Reporter | ||
Comment 54•14 years ago
|
||
Attachment #545238 -
Flags: review?(jonas)
Reporter | ||
Updated•14 years ago
|
Attachment #536102 -
Attachment is obsolete: true
Reporter | ||
Updated•14 years ago
|
Attachment #536103 -
Attachment is obsolete: true
Reporter | ||
Updated•14 years ago
|
Attachment #536104 -
Attachment is obsolete: true
Reporter | ||
Updated•14 years ago
|
Attachment #536105 -
Attachment is obsolete: true
Reporter | ||
Comment 55•14 years ago
|
||
(In reply to comment #42)
> 4. Make the XMLContentSink merge cdatasection text data with preceding text
> data just like it does for normal text data. However make it also set the
> mozSerializeAsCDATA property to true when creating the textnode if any of
> the data came from a CDATA section in the markup.
I think I would somewhat prefer not merging, but I can change it if you prefer.
> 6. Add a warning whenever createCDATASection is called stating that it's
> deprecated and will be removed. (We'll probably remove it for FF8 or some
> such).
Not done yet.
Also, I've drafted Text.serializeAsCDATA at <http://html5.org/specs/dom-parsing.html#extensions-to-the-text-interface>.
I'm aware not everyone is happy with this approach and certainly welcome better solutions.
Comment 56•14 years ago
|
||
just don't remove CDATASection?
It is not clear to me why we have to remove it.
Comment on attachment 545233 [details] [diff] [review]
, part a - Implement Text.mozSerializeAsCDATA;
Review of attachment 545233 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good otherwise, but would like to see a new patch as this'll change the patch a lot.
::: content/base/src/nsGenericDOMDataNode.h
@@ +370,5 @@
> virtual nsGenericDOMDataNode *CloneDataNode(nsINodeInfo *aNodeInfo,
> PRBool aCloneText) const = 0;
>
> nsTextFragment mText;
> + bool mIsCDATA;
Don't add this member as it'll increase the size of all textnodes. First off, just implement serializeAsCDATA on nsTextNode as to keep nsGenericDOMDataNode smaller. Second, implement it by reusing one of the nsINode flags which is only applicable to elements, there should be plenty. NODE_HAS_ACCESSKEY looks like a good candidate for example.
Attachment #545233 -
Flags: review?(jonas) → review-
Comment on attachment 545234 [details] [diff] [review]
, part b - Make Document.createCDATASection return Text;
Review of attachment 545234 [details] [diff] [review]:
-----------------------------------------------------------------
Change the IID on nsIDOMDocument. r=me with that
Attachment #545234 -
Flags: review?(jonas) → review+
Comment on attachment 545238 [details] [diff] [review]
, part f - Remove dead code;
Review of attachment 545238 [details] [diff] [review]:
-----------------------------------------------------------------
Also remove cdataTagName from nsGkAtoms. r=me with that.
Attachment #545238 -
Flags: review?(jonas) → review+
Comment on attachment 545237 [details] [diff] [review]
, part e - Remove CDATASection and adjust tests;
Review of attachment 545237 [details] [diff] [review]:
-----------------------------------------------------------------
++
Attachment #545237 -
Flags: review?(jonas) → review+
Comment on attachment 545236 [details] [diff] [review]
, part d - Pay attention to Text.mozSerializeAsCDATA when serializing;
Review of attachment 545236 [details] [diff] [review]:
-----------------------------------------------------------------
++
Attachment #545236 -
Flags: review?(jonas) → review+
Comment on attachment 545235 [details] [diff] [review]
, part c - Make the XML content sink create Text nodes;
Review of attachment 545235 [details] [diff] [review]:
-----------------------------------------------------------------
I do think that we should merge adjacent text nodes. I think it's good and simpler for authors if they can rely on that all adjacent text appears as a single node.
Attachment #545235 -
Flags: review?(jonas) → review-
Comment 63•12 years ago
|
||
Any progress on this case?
I test last stable Firefox (and Nightly), Chrome and IE9 and all this browser still support CDATASection node and method document.createCDATASection().
New variant; Text node and it's serializeAsCDATA attribute looks still not work.
<script type = "text/javascript">
var new_XML = new DOMParser().parseFromString("<xml></xml>", "application/xml");
var cdata = new_XML.createCDATASection("Some <CDATA> data & then some");
var txt = new_XML.createTextNode("Some <CDATA> data & then some");
txt.serializeAsCDATA = true;
var serializer = new XMLSerializer();
alert(serializer.serializeToString(cdata)); // all correct
alert(serializer.serializeToString(txt)) // trates as normal Text node
</script>
Comment 64•11 years ago
|
||
I was directed here from https://www.w3.org/Bugs/Public/show_bug.cgi?id=27386#c13
I guess none of these changes ever landed?
Reporter | ||
Comment 65•11 years ago
|
||
That's correct.
Reporter | ||
Updated•10 years ago
|
Assignee: Ms2ger → nobody
Status: ASSIGNED → NEW
Comment 66•5 years ago
|
||
Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority and severity.
If you have reason to believe this is wrong, please write a comment and ni :jstutte.
Severity: normal → S4
Priority: -- → P5
Comment 67•4 years ago
|
||
This feature got reinstated.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•