Chromium Code Reviews Chromium Code Reviews
Issue 115693010:
Fix Range.createContextualFragment for non-Element contexts. (Closed)
Base URL: https://chromium.googlesource.com/chromium/blink.git@master| Index: Source/core/dom/Range.cpp |
| diff --git a/Source/core/dom/Range.cpp b/Source/core/dom/Range.cpp |
| index 9189f403bbb65bda280a75d7fa493fb648135f57..a5934ef734fa7c55e4d01069432baae1e8614b64 100644 |
| --- a/Source/core/dom/Range.cpp |
| +++ b/Source/core/dom/Range.cpp |
| @@ -40,6 +40,7 @@ |
| #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" |
| @@ -54,7 +55,6 @@ |
| namespace WebCore { |
| using namespace std; |
| -using namespace HTMLNames; |
| DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, rangeCounter, ("Range")); |
| @@ -979,13 +979,51 @@ String Range::text() const |
| 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 Ra
pwnall-personal
2014/06/04 06:54:37
I re-read the spec, and I'm pretty sure we can get
|
| + return nullptr; |
| + } |
| + } |
| + |
| + // 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
pwnall-personal
2014/06/04 06:54:37
Done.
Sorry, for some reason I confused XML and X
|
| + 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 migh
pwnall-personal
2014/06/04 06:54:38
You're right. Thank you very much for pointing tha
|
| + 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
pwnall-personal
2014/06/04 06:54:38
At least as far as the spec is concerned, the fake
|
| + } |
| + } 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
pwnall-personal
2014/06/04 06:54:38
Now the exception is only thrown in one place, if
|
| + 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 e
pwnall-personal
2014/06/04 06:54:38
Done.
|
| - RefPtrWillBeRawPtr<DocumentFragment> fragment = WebCore::createContextualFragment(markup, toHTMLElement(element), AllowScriptingContentAndDoNotMarkAlreadyStarted, exceptionState); |
| + // Steps 3, 4, 5. |
| + RefPtrWillBeRawPtr<DocumentFragment> fragment = WebCore::createContextualFragment(markup, element.get(), AllowScriptingContentAndDoNotMarkAlreadyStarted, exceptionState); |
| if (!fragment) |
| return nullptr; |
