Fix Range.createContextualFragment for non-Element contexts.

Source/core/dom/Range.cpp

 /*
  * (C) 1999 Lars Knoll (knoll@kde.org)
  * (C) 2000 Gunnstein Lye (gunnstein@netcom.no)
  * (C) 2000 Frederik Holljen (frederik.holljen@hig.no)
  * (C) 2001 Peter Kelly (pmk@post.com)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Motorola Mobility. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 22 matching lines @@
 #include "core/dom/Node.h"
 #include "core/dom/NodeTraversal.h"
 #include "core/dom/NodeWithIndex.h"
 #include "core/dom/ProcessingInstruction.h"
 #include "core/dom/Text.h"
 #include "core/editing/TextIterator.h"
 #include "core/editing/VisiblePosition.h"
 #include "core/editing/VisibleUnits.h"
 #include "core/editing/markup.h"
 #include "core/events/ScopedEventQueue.h"
+#include "core/html/HTMLBodyElement.h"
 #include "core/html/HTMLElement.h"
 #include "core/rendering/RenderBoxModelObject.h"
 #include "core/rendering/RenderText.h"
 #include "platform/geometry/FloatQuad.h"
 #include "wtf/RefCountedLeakCounter.h"
 #include "wtf/text/CString.h"
 #include "wtf/text/StringBuilder.h"
 #ifndef NDEBUG
 #include <stdio.h>
 #endif
 
 namespace WebCore {
 
 using namespace std;
-using namespace HTMLNames;
 
 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, rangeCounter, ("Range"));
 
 inline Range::Range(Document& ownerDocument)
     : m_ownerDocument(&ownerDocument)
     , m_start(m_ownerDocument)
     , m_end(m_ownerDocument)
 {
 #ifndef NDEBUG
     rangeCounter.increment();
@@ skipping 904 matching lines @@
 {
     // We need to update layout, since plainText uses line boxes in the render tree.
     // FIXME: As with innerText, we'd like this to work even if there are no render objects.
     m_start.container()->document().updateLayout();
 
     return plainText(this);
 }
 
 PassRefPtrWillBeRawPtr<DocumentFragment> Range::createContextualFragment(const String& markup, ExceptionState& exceptionState)
 {
-    Node* element = m_start.container()->isElementNode() ? m_start.container() : m_start.container()->parentNode();
-    if (!element || !element->isHTMLElement()) {
-        exceptionState.throwDOMException(NotSupportedError, "The range's container must be an HTML element.");
-        return nullptr;
+    // Algorithm: http://domparsing.spec.whatwg.org/#extensions-to-the-range-interface
+
+    Node* node = m_start.container();
+
+    // Step 1.
+    RefPtrWillBeRawPtr<Element> element;
+    if (!m_start.offset() && (node->isDocumentNode() || node->isDocumentFragment())) {
+        element = nullptr;
+    } else if (node->isElementNode()) {
+        element = toElement(node);
+    } else {
+        element = node->parentElement();
+        if (!element) {
+            exceptionState.throwDOMException(NotSupportedError, "The range's container must be an Element, Document, or DocumentFragment.");

[eseidel, 2014/06/03 00:32:46]

We could be more specific here.  It's just that Range is rooted in a detached
node?

[pwnall-personal, 2014/06/04 06:54:37]

I re-read the spec, and I'm pretty sure we can get away without throwing here. I
updated the code and added test cases.

+            return nullptr;
+        }
     }
 
-    RefPtrWillBeRawPtr<DocumentFragment> fragment = WebCore::createContextualFragment(markup, toHTMLElement(element), AllowScriptingContentAndDoNotMarkAlreadyStarted, exceptionState);
+    // Step 2.
+    if (!element || isHTMLHtmlElement(element)) {
+        Document& document = node->document();
+
+        if (document.isSVGDocument()) {

[eseidel, 2014/06/03 00:27:02]

I think you want to reverse this, no?  If it's an HTML doc, then check for the
body, otherwise assume documentElement?

[pwnall-personal, 2014/06/04 06:54:37]

Done.

Sorry, for some reason I confused XML and XHTML documents, and was worried that
an SVG document will return true for isXHTMLDocument(). Live and learn :/

+            element = document.documentElement();
+        } else {
+            // Optimization over spec: try to reuse the existing <body> element, if it is available.
+            ASSERT(document.isHTMLDocument() || document.isXHTMLDocument());
+            element = document.body();
+        }
+
+        if (!element) {
+            // SVG documents always have an <svg> root element, should never need a fake node.

[eseidel, 2014/06/03 00:27:02]

I'm not sure that's true.  an empty .svg file might not have a documentElement? 
I'm not sure.

[pwnall-personal, 2014/06/04 06:54:38]

You're right. Thank you very much for pointing that out!
Test added, code fixed.

+            ASSERT(!document.isSVGDocument());
+            element = HTMLBodyElement::create(document);

[eseidel, 2014/06/03 00:32:46]

I don't really understand what this is going to do.  Is the idea that this would
return the string "<body></body>"?  Can we just return that explicitly instead
of creating a fake element?

[pwnall-personal, 2014/06/04 06:54:38]

At least as far as the spec is concerned, the fake <body> serves as a context
for the HTML parser.

I admit that I haven't looked at the parser to see whether this makes a
difference or not. Do you want me to dig deeper here?

+        }
+    } else {
+        if (!element->isHTMLElement() && !element->isSVGElement()) {
+            exceptionState.throwDOMException(NotSupportedError, "The range's container must be an Element, Document, or DocumentFragment.");

[eseidel, 2014/06/03 00:27:02]

We throw this in multiple places?  Why even bother with the "else" here?  We
could unconditionally check "if (!element)" since element is only set if those
conditions are met, no?

[pwnall-personal, 2014/06/04 06:54:38]

Now the exception is only thrown in one place, if the range's document isn't
HTML / SVG. As far as I know, this implies the document is XML. I added a couple
of tests that cover this code.

FWIW, according to the spec, an exception should never be thrown. At the same
time, I'd rather not try to tackle the XML case in the same patch. It's already
one small step in the right direction.

+            return nullptr;
+        }
+    }
+    ASSERT(element && (element->isHTMLElement() || element->isSVGElement()));

[eseidel, 2014/06/03 00:32:46]

If you bumped the other if (!element) out of the else block above then we could
just remove this.

[pwnall-personal, 2014/06/04 06:54:38]

Done.

+
+    // Steps 3, 4, 5.
+    RefPtrWillBeRawPtr<DocumentFragment> fragment = WebCore::createContextualFragment(markup, element.get(), AllowScriptingContentAndDoNotMarkAlreadyStarted, exceptionState);
     if (!fragment)
         return nullptr;
 
     return fragment.release();
 }
 
 
 void Range::detach()
 {
     // This is now a no-op as per the DOM specification.
@@ skipping 689 matching lines @@
 
 void showTree(const WebCore::Range* range)
 {
     if (range && range->boundaryPointsValid()) {
         range->startContainer()->showTreeAndMark(range->startContainer(), "S", range->endContainer(), "E");
         fprintf(stderr, "start offset: %d, end offset: %d\n", range->startOffset(), range->endOffset());
     }
 }
 
 #endif