Closed Bug 655512 Opened 8 years ago Closed 8 years ago

Forward implementation of nsIDOMDocument from nsHTMLDocument to nsDocument

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Details

Attachments

(1 file)

Attached patch Patch v1Splinter Review
The manual forwarding is hurting my attempts to get rid of some nsIDOMDocument* interfaces.
Attachment #530869 - Flags: review?(Olli.Pettay)
Flags: in-testsuite-
Attachment #530869 - Flags: review?(Olli.Pettay) → review+
Whiteboard: [needs review] → [needs landing]
http://hg.mozilla.org/mozilla-central/rev/11339a517f79
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla6
Wonderful :-( Thank you so much, you just totally broke BlueGriffon, giving
it its first _major_ loss of functionality.

Why did you forbid CDATASections and PIs in HTML???? There are tons of use
case, the first of them being PHP pseudo-PIs !!!

I am officially requesting a backout of this changeset.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
There is no black-box-detectable change in this changeset.
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
(In reply to comment #3)
> There is no black-box-detectable change in this changeset.

I don't understand this comment. Care to explain?
Daniel, in Fx4, 192, etc 
htmldocument.createCDATASection("foo") throws.
So it does with Nightlies.

What is the change to the behavior?
The old code was
CreateCDATASection(const nsAString& aData, nsIDOMCDATASection** aReturn)
{
  if (!IsHTML()) {
    return nsDocument::CreateCDATASection(aData, aReturn);
  }
  // There are no CDATASections in HTML
  *aReturn = nsnull;
   return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
}


The new code is 
CreateCDATASection(const nsAString& aData, nsIDOMCDATASection** aReturn)
{
  NS_ENSURE_ARG_POINTER(aReturn);
  *aReturn = nsnull;

  if (IsHTML()) {
    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
  }
...
D'OH. So this never worked ?!? Then one of my tests of the Toolkits add-on
for BlueGriffon failed w/o notice. I need tests for the tests apparently :-(
If that's the case I _deeply_ apologize.

But the root of the problem still stands:

1. if I understand correctly the code, createCDATASection then throws
   for all nsHTMLDocuments.

2. in that case, it throws also for XHTML1 documents

3. but for validation reasons, it it *extremely* important that editors
   encapsulate the contents of a <script> element inside a CDATA section

4. otherwise, creating a <script> element and its content text node will
   result in the transliteration of all instances of < into &lt; and &
   into &amp;, making the script extremely painful to manipulate

5. I remind you the whole web is full of tutorials strongly recommending
   to encapsulate XHTML <script> contents in CDATA sections with the
   CDATA section start and end tag commented out by a JavaScript comment

I really need a solution here. Making createCDATASection throw for HTML
documents is a total blocker to me.
Similarly, making createProcessingInstruction throw for HTML document is
a total blocker for PHP pseudo-instructions in HTML for my editor.
(In reply to comment #7)
> But the root of the problem still stands:
> 
> 1. if I understand correctly the code, createCDATASection then throws
>    for all nsHTMLDocuments.
> 
> 2. in that case, it throws also for XHTML1 documents
It doesn't throw for XHTML document, only HTML

> I really need a solution here. Making createCDATASection throw for HTML
> documents is a total blocker to me.
It has thrown for HTML documents for ages.
(In reply to comment #8)
> It has thrown for HTML documents for ages.
Apparently since 1998
I discussed that with Ms2Ger : the extension of a XHTML file is unpredictable
and is independant of the MIME type. So when a XHTML file is locally saved
under a *.html file, I can't create a CDATASection... Blocker.
You can, in a hackish way.
var cdata = document.implementation.createDocument("", "", null).createCDATASection("Foobar");
elementInHTMLDocument.appendChild(cdata);


But anyway, the behavior hasn't changed for ages.
You need to log in before you can comment on or make changes to this bug.