Source/core/dom/Range.cpp - Issue 115693010: Fix Range.createContextualFragment for non-Element contexts.

Side by Side Diff: Source/core/dom/Range.cpp

Issue 115693010: Fix Range.createContextualFragment for non-Element contexts. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Addressed feedback. Created 6 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 /*	1 /*

2 * (C) 1999 Lars Knoll (knoll@kde.org)	2 * (C) 1999 Lars Knoll (knoll@kde.org)

3 * (C) 2000 Gunnstein Lye (gunnstein@netcom.no)	3 * (C) 2000 Gunnstein Lye (gunnstein@netcom.no)

4 * (C) 2000 Frederik Holljen (frederik.holljen@hig.no)	4 * (C) 2000 Frederik Holljen (frederik.holljen@hig.no)

5 * (C) 2001 Peter Kelly (pmk@post.com)	5 * (C) 2001 Peter Kelly (pmk@post.com)

6 * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All r ights reserved.	6 * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All r ights reserved.

7 * Copyright (C) 2011 Motorola Mobility. All rights reserved.	7 * Copyright (C) 2011 Motorola Mobility. All rights reserved.

8 *	8 *

9 * This library is free software; you can redistribute it and/or	9 * This library is free software; you can redistribute it and/or

10 * modify it under the terms of the GNU Library General Public	10 * modify it under the terms of the GNU Library General Public

(...skipping 22 matching lines...) Expand all Loading...
33 #include "core/dom/Node.h"	33 #include "core/dom/Node.h"

34 #include "core/dom/NodeTraversal.h"	34 #include "core/dom/NodeTraversal.h"

35 #include "core/dom/NodeWithIndex.h"	35 #include "core/dom/NodeWithIndex.h"

36 #include "core/dom/ProcessingInstruction.h"	36 #include "core/dom/ProcessingInstruction.h"

37 #include "core/dom/Text.h"	37 #include "core/dom/Text.h"

38 #include "core/editing/TextIterator.h"	38 #include "core/editing/TextIterator.h"

39 #include "core/editing/VisiblePosition.h"	39 #include "core/editing/VisiblePosition.h"

40 #include "core/editing/VisibleUnits.h"	40 #include "core/editing/VisibleUnits.h"

41 #include "core/editing/markup.h"	41 #include "core/editing/markup.h"

42 #include "core/events/ScopedEventQueue.h"	42 #include "core/events/ScopedEventQueue.h"

	43 #include "core/html/HTMLBodyElement.h"

43 #include "core/html/HTMLElement.h"	44 #include "core/html/HTMLElement.h"

44 #include "core/rendering/RenderBoxModelObject.h"	45 #include "core/rendering/RenderBoxModelObject.h"

45 #include "core/rendering/RenderText.h"	46 #include "core/rendering/RenderText.h"

46 #include "platform/geometry/FloatQuad.h"	47 #include "platform/geometry/FloatQuad.h"

47 #include "wtf/RefCountedLeakCounter.h"	48 #include "wtf/RefCountedLeakCounter.h"

48 #include "wtf/text/CString.h"	49 #include "wtf/text/CString.h"

49 #include "wtf/text/StringBuilder.h"	50 #include "wtf/text/StringBuilder.h"

50 #ifndef NDEBUG	51 #ifndef NDEBUG

51 #include <stdio.h>	52 #include <stdio.h>

52 #endif	53 #endif

53	54

54 namespace WebCore {	55 namespace WebCore {

55	56

56 using namespace std;	57 using namespace std;

57 using namespace HTMLNames;

58	58

59 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, rangeCounter, ("Range"));	59 DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, rangeCounter, ("Range"));

60	60

61 inline Range::Range(Document& ownerDocument)	61 inline Range::Range(Document& ownerDocument)

62 : m_ownerDocument(&ownerDocument)	62 : m_ownerDocument(&ownerDocument)

63 , m_start(m_ownerDocument)	63 , m_start(m_ownerDocument)

64 , m_end(m_ownerDocument)	64 , m_end(m_ownerDocument)

65 {	65 {

66 #ifndef NDEBUG	66 #ifndef NDEBUG

67 rangeCounter.increment();	67 rangeCounter.increment();

(...skipping 904 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
972 {	972 {

973 // We need to update layout, since plainText uses line boxes in the render t ree.	973 // We need to update layout, since plainText uses line boxes in the render t ree.

974 // FIXME: As with innerText, we'd like this to work even if there are no ren der objects.	974 // FIXME: As with innerText, we'd like this to work even if there are no ren der objects.

975 m_start.container()->document().updateLayout();	975 m_start.container()->document().updateLayout();

976	976

977 return plainText(this);	977 return plainText(this);

978 }	978 }

979	979

980 PassRefPtrWillBeRawPtr<DocumentFragment> Range::createContextualFragment(const S tring& markup, ExceptionState& exceptionState)	980 PassRefPtrWillBeRawPtr<DocumentFragment> Range::createContextualFragment(const S tring& markup, ExceptionState& exceptionState)

981 {	981 {

982 Node* element = m_start.container()->isElementNode() ? m_start.container() : m_start.container()->parentNode();	982 // Algorithm: http://domparsing.spec.whatwg.org/#extensions-to-the-range-int erface

983 if (!element \|\| !element->isHTMLElement()) {	983

984 exceptionState.throwDOMException(NotSupportedError, "The range's contain er must be an HTML element.");	984 Node* startContainer = m_start.container();

	985 if (!startContainer) {

	986 exceptionState.throwDOMException(InvalidStateError, "The range must have a container.");

985 return nullptr;	987 return nullptr;

986 }	988 }

987	989

988 RefPtrWillBeRawPtr<DocumentFragment> fragment = WebCore::createContextualFra gment(markup, toHTMLElement(element), AllowScriptingContentAndDoNotMarkAlreadySt arted, exceptionState);	990 // Step 1.

	991 Node* element;

	992 if (!m_start.offset() && (m_start.container()->isDocumentNode() \|\| m_start.c ontainer()->isDocumentFragment())) {

	993 element = nullptr;

	994 } else if (startContainer->isElementNode()) {

	995 element = startContainer;

	996 } else {

	997 element = startContainer->parentNode();

	998 if (!element \|\| !element->isElementNode()) {

	999 exceptionState.throwDOMException(NotSupportedError, "The range's con tainer must be an Element, Document, or DocumentFragment.");

	1000 return nullptr;

	1001 }

	1002 }

	1003 ASSERT(!element \|\| element->isElementNode());

	1004

	1005 // Step 2.

	1006 RefPtrWillBeRawPtr<Element> fakeBody = nullptr;

	1007 if (!element \|\| isHTMLHtmlElement(element)) {

	1008 Document& document = startContainer->document();

	1009

	1010 if (document.isSVGDocument()) {
	eseidel 2014/06/02 16:51:28 I would have just written this as a ternary to sav I would have just written this as a ternary to save space: element = document.isSVGDocument() ? document.documentElement() : document.body(); pwnall-personal 2014/06/03 00:09:01 I was worried about that because the comment about Show quoted text On 2014/06/02 16:51:28, eseidel wrote: > I would have just written this as a ternary to save space: > > element = document.isSVGDocument() ? document.documentElement() : > document.body(); I was worried about that because the comment about reusing <body> only applies to the 2nd part of the branch. The comment seems superfluous as it is, I tried to make it clearer that it matters because we're being a bit more clever than the spec algorithm here, to avoid an unnecessary Element creation.
	1011 element = document.documentElement();

	1012 } else {

	1013 // Try to use the existing <body> element, if it is available.

	1014 element = document.body();

	1015 }

	1016

	1017 if (!element) {

	1018 // SVG documents always have an <svg> root element, should never nee d a fake node.

	1019 ASSERT(document.isHTMLDocument() \|\| (document.isXHTMLDocument() && ! document.isSVGDocument()));
	eseidel 2014/06/02 16:51:28 Why not move this ASSERT into the document.documen Why not move this ASSERT into the document.documentElement() branch above (then you wouldn't use a ternary of course). pwnall-personal 2014/06/03 00:09:01 I revised the asserts to express my assumptions th Show quoted text On 2014/06/02 16:51:28, eseidel wrote: > Why not move this ASSERT into the document.documentElement() branch above (then > you wouldn't use a ternary of course). I revised the asserts to express my assumptions that might not immediately come out from the code. Is this better?
	1020 fakeBody = HTMLBodyElement::create(document);

	1021 element = fakeBody.get();
	eseidel 2014/06/02 16:51:28 It's generally dangerous to put RefCounted objects It's generally dangerous to put RefCounted objects in raw pointers. It would be easy for someone to refactor this method and have fakeBody die early. I would make "element" a RefPtr. also, why is it called "element" if it's a Node? pwnall-personal* 2014/06/03 00:09:01 Done and done. Thank you! Show quoted text On 2014/06/02 16:51:28, eseidel wrote: > It's generally dangerous to put RefCounted objects in raw pointers. It would be > easy for someone to refactor this method and have fakeBody die early. I would > make "element" a RefPtr. also, why is it called "element" if it's a Node*? Done and done. Thank you!
	1022 }

	1023 } else {

	1024 if (!element->isHTMLElement() && !element->isSVGElement()) {

	1025 exceptionState.throwDOMException(NotSupportedError, "The range's con tainer must be an Element, Document, or DocumentFragment.");

	1026 return nullptr;

	1027 }

	1028 }

	1029 ASSERT(element && (element->isHTMLElement() \|\| element->isSVGElement()));

	1030

	1031 // Steps 3, 4, 5.

	1032 RefPtrWillBeRawPtr<DocumentFragment> fragment = WebCore::createContextualFra gment(markup, toElement(element), AllowScriptingContentAndDoNotMarkAlreadyStarte d, exceptionState);

989 if (!fragment)	1033 if (!fragment)

990 return nullptr;	1034 return nullptr;

991	1035

992 return fragment.release();	1036 return fragment.release();

993 }	1037 }

994	1038

995	1039

996 void Range::detach()	1040 void Range::detach()

997 {	1041 {

998 // This is now a no-op as per the DOM specification.	1042 // This is now a no-op as per the DOM specification.

(...skipping 689 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
1688	1732

1689 void showTree(const WebCore::Range* range)	1733 void showTree(const WebCore::Range* range)

1690 {	1734 {

1691 if (range && range->boundaryPointsValid()) {	1735 if (range && range->boundaryPointsValid()) {

1692 range->startContainer()->showTreeAndMark(range->startContainer(), "S", r ange->endContainer(), "E");	1736 range->startContainer()->showTreeAndMark(range->startContainer(), "S", r ange->endContainer(), "E");

1693 fprintf(stderr, "start offset: %d, end offset: %d\n", range->startOffset (), range->endOffset());	1737 fprintf(stderr, "start offset: %d, end offset: %d\n", range->startOffset (), range->endOffset());

1694 }	1738 }

1695 }	1739 }

1696	1740

1697 #endif	1741 #endif

OLD	NEW

« no previous file with comments | « LayoutTests/http/tests/dom/resources/svg-document.svg ('k') | Source/core/editing/markup.h » ('j') | Source/core/editing/markup.cpp » ('J')