Attachment 141660 Details for Bug 86341 – Patch

[patch] Patch

bug-86341-20120514165001.patch (text/plain), 11.86 KB, created by Shinya Kawanaka on 2012-05-14 00:50:02 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Shinya Kawanaka

Created: 2012-05-14 00:50:02 PDT

Size: 11.86 KB

patch

obsolete

>Subversion Revision: 116911
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 98599c8f76b090a05728dabb2f3cda88046ad5ff..e2112f924eb96d4e03ddbdb00246e29e998d6a77 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2012-05-14  Shinya Kawanaka  <shinyak@chromium.org>
>+
>+        document.execCommand('Indent') in the direct child of ShadowRoot causes a crash.
>+        https://bugs.webkit.org/show_bug.cgi?id=86341
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When document.execCommand('Indent') is executed in the direct child of ShadowRoot,
>+        the scope variable of TextIterator becomes ShadowRoot. However, an element is assumed for it,
>+        so a crash happended.
>+
>+        This patch enables TextIterator to have ShadowRoot as a scope variable.
>+
>+        Test: editing/shadow/execcommand-indent-in-shadow.html
>+
>+        * editing/ApplyBlockElementCommand.cpp:
>+        (WebCore::ApplyBlockElementCommand::doApply):
>+        * editing/InsertListCommand.cpp:
>+        (WebCore::InsertListCommand::doApply):
>+        * editing/TextIterator.cpp:
>+        (WebCore::TextIterator::rangeFromLocationAndLength):
>+        * editing/TextIterator.h:
>+        (TextIterator):
>+        * editing/htmlediting.cpp:
>+        (WebCore::indexForVisiblePosition):
>+        (WebCore::visiblePositionForIndex):
>+        * editing/htmlediting.h:
>+        (WebCore):
>+
> 2012-05-13  Darin Adler  <darin@apple.com>
> 
>         Roll out local changes accidentally landed in r116905.
>diff --git a/Source/WebKit/chromium/ChangeLog b/Source/WebKit/chromium/ChangeLog
>index f8a50bc503369dcb389f9c897f1abfeb3c8081bb..c5eb4b8b70926f6b9710f463d68970b198245f7c 100644
>--- a/Source/WebKit/chromium/ChangeLog
>+++ b/Source/WebKit/chromium/ChangeLog
>@@ -1,3 +1,13 @@
>+2012-05-14  Shinya Kawanaka  <shinyak@chromium.org>
>+
>+        document.execCommand('Indent') in the direct child of ShadowRoot causes a crash.
>+        https://bugs.webkit.org/show_bug.cgi?id=86341
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * src/WebRange.cpp:
>+        (WebKit::WebRange::fromDocumentRange):
>+
> 2012-05-12  Sheriff Bot  <webkit.review.bot@gmail.com>
> 
>         Unreviewed, rolling out r116812.
>diff --git a/Source/WebCore/editing/ApplyBlockElementCommand.cpp b/Source/WebCore/editing/ApplyBlockElementCommand.cpp
>index 952dc097b9759df14bc902109235296ce02a94ea..73df5895114ad35b6c82286292f9dd3681ffe8e6 100644
>--- a/Source/WebCore/editing/ApplyBlockElementCommand.cpp
>+++ b/Source/WebCore/editing/ApplyBlockElementCommand.cpp
>@@ -80,9 +80,9 @@ void ApplyBlockElementCommand::doApply()
>     VisiblePosition endOfSelection = selection.visibleEnd();
>     ASSERT(!startOfSelection.isNull());
>     ASSERT(!endOfSelection.isNull());
>-    RefPtr<Element> startScope;
>+    RefPtr<Node> startScope;
>     int startIndex = indexForVisiblePosition(startOfSelection, startScope);
>-    RefPtr<Element> endScope;
>+    RefPtr<Node> endScope;
>     int endIndex = indexForVisiblePosition(endOfSelection, endScope);
> 
>     formatSelection(startOfSelection, endOfSelection);
>diff --git a/Source/WebCore/editing/InsertListCommand.cpp b/Source/WebCore/editing/InsertListCommand.cpp
>index e4e35782685442f004020cde7bcdaeee59468cd1..0974592596a007bcd7c28d39512b48c20199a698 100644
>--- a/Source/WebCore/editing/InsertListCommand.cpp
>+++ b/Source/WebCore/editing/InsertListCommand.cpp
>@@ -152,7 +152,7 @@ void InsertListCommand::doApply()
>                 // FIXME: This is an inefficient way to keep selection alive because indexForVisiblePosition walks from
>                 // the beginning of the document to the endOfSelection everytime this code is executed.
>                 // But not using index is hard because there are so many ways we can lose selection inside doApplyForSingleParagraph.
>-                RefPtr<Element> scope;
>+                RefPtr<Node> scope;
>                 int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, scope);
>                 doApplyForSingleParagraph(forceCreateList, listTag, currentSelection.get());
>                 if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
>diff --git a/Source/WebCore/editing/TextIterator.cpp b/Source/WebCore/editing/TextIterator.cpp
>index 85b5595145097dc2f7e547f2478e535671790ea1..897eec3b11e853c25f8ab83a85487ffd0886345e 100644
>--- a/Source/WebCore/editing/TextIterator.cpp
>+++ b/Source/WebCore/editing/TextIterator.cpp
>@@ -2402,7 +2402,7 @@ PassRefPtr<Range> TextIterator::subrange(Range* entireRange, int characterOffset
>     return characterSubrange(entireRangeIterator, characterOffset, characterCount);
> }
> 
>-PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Element* scope, int rangeLocation, int rangeLength, bool forSelectionPreservation)
>+PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(Node* scope, int rangeLocation, int rangeLength, bool forSelectionPreservation)
> {
>     RefPtr<Range> resultRange = scope->document()->createRange();
> 
>diff --git a/Source/WebCore/editing/TextIterator.h b/Source/WebCore/editing/TextIterator.h
>index bfcfae3566e4c0a7a37e0b3b4d732c6b93924631..ccf670791628491613a46b3a7a0c7cb6ec579e97 100644
>--- a/Source/WebCore/editing/TextIterator.h
>+++ b/Source/WebCore/editing/TextIterator.h
>@@ -100,7 +100,7 @@ public:
>     Node* node() const;
>      
>     static int rangeLength(const Range*, bool spacesForReplacedElements = false);
>-    static PassRefPtr<Range> rangeFromLocationAndLength(Element* scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements = false);
>+    static PassRefPtr<Range> rangeFromLocationAndLength(Node* scope, int rangeLocation, int rangeLength, bool spacesForReplacedElements = false);
>     static bool getLocationAndLengthFromRange(Element* scope, const Range*, size_t& location, size_t& length);
>     static PassRefPtr<Range> subrange(Range* entireRange, int characterOffset, int characterCount);
>     
>diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp
>index 3b1d44c02e6c6d3cd1582d78f5bba68e703790c0..d1588c0d5caac3d797a8656bad02181bca627d6a 100644
>--- a/Source/WebCore/editing/htmlediting.cpp
>+++ b/Source/WebCore/editing/htmlediting.cpp
>@@ -1084,7 +1084,7 @@ VisibleSelection selectionForParagraphIteration(const VisibleSelection& original
> // opertion is unreliable. TextIterator's TextIteratorEmitsCharactersBetweenAllVisiblePositions mode needs to be fixed, 
> // or these functions need to be changed to iterate using actual VisiblePositions.
> // FIXME: Deploy these functions everywhere that TextIterators are used to convert between VisiblePositions and indices.
>-int indexForVisiblePosition(const VisiblePosition& visiblePosition, RefPtr<Element>& scope)
>+int indexForVisiblePosition(const VisiblePosition& visiblePosition, RefPtr<Node>& scope)
> {
>     if (visiblePosition.isNull())
>         return 0;
>@@ -1095,8 +1095,8 @@ int indexForVisiblePosition(const VisiblePosition& visiblePosition, RefPtr<Eleme
> 
>     if (shadowRoot) {
>         // Use the shadow root for form elements, since TextIterators will not enter shadow content.
>-        ASSERT(shadowRoot->isElementNode());
>-        scope = static_cast<Element*>(shadowRoot);
>+        ASSERT(shadowRoot->isElementNode() || shadowRoot->isShadowRoot());
>+        scope = shadowRoot;
>     } else
>         scope = document->documentElement();
> 
>@@ -1105,7 +1105,7 @@ int indexForVisiblePosition(const VisiblePosition& visiblePosition, RefPtr<Eleme
>     return TextIterator::rangeLength(range.get(), true);
> }
> 
>-VisiblePosition visiblePositionForIndex(int index, Element *scope)
>+VisiblePosition visiblePositionForIndex(int index, Node* scope)
> {
>     RefPtr<Range> range = TextIterator::rangeFromLocationAndLength(scope, index, 0, true);
>     // Check for an invalid index. Certain editing operations invalidate indices because 
>diff --git a/Source/WebCore/editing/htmlediting.h b/Source/WebCore/editing/htmlediting.h
>index c2cf4563bbcf0bd350f6e41d3bbac8254212fa81..867aeb0261bcd1b28e048361b6a956c83f2f5649 100644
>--- a/Source/WebCore/editing/htmlediting.h
>+++ b/Source/WebCore/editing/htmlediting.h
>@@ -180,8 +180,8 @@ bool lineBreakExistsAtVisiblePosition(const VisiblePosition&);
>     
> int comparePositions(const VisiblePosition&, const VisiblePosition&);
> 
>-int indexForVisiblePosition(const VisiblePosition&, RefPtr<Element>& scope);
>-VisiblePosition visiblePositionForIndex(int index, Element *scope);
>+int indexForVisiblePosition(const VisiblePosition&, RefPtr<Node>& scope);
>+VisiblePosition visiblePositionForIndex(int index, Node* scope);
> 
> // -------------------------------------------------------------------------
> // Range
>diff --git a/Source/WebKit/chromium/src/WebRange.cpp b/Source/WebKit/chromium/src/WebRange.cpp
>index be24dd7931156bfb661994c8b7f6a91f9c4c77ff..18e0325c6931c498c477864b8b6d566094823f9f 100644
>--- a/Source/WebKit/chromium/src/WebRange.cpp
>+++ b/Source/WebKit/chromium/src/WebRange.cpp
>@@ -32,6 +32,7 @@
> #include "WebRange.h"
> 
> #include "Document.h"
>+#include "Element.h"
> #include "Frame.h"
> #include "FrameView.h"
> #include "Range.h"
>@@ -97,7 +98,7 @@ WebRange WebRange::fromDocumentRange(WebFrame* frame, int start, int length)
> {
>     WebCore::Frame* webFrame = static_cast<WebFrameImpl*>(frame)->frame();
>     Element* selectionRoot = webFrame->selection()->rootEditableElement();
>-    Element* scope = selectionRoot ? selectionRoot : webFrame->document()->documentElement();
>+    Node* scope = selectionRoot ? selectionRoot : webFrame->document()->documentElement();
>     return TextIterator::rangeFromLocationAndLength(scope, start, length);
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fe6f075164011b4bc7714cfde8821eb829dce2bc..a3c8b9a4ab8e97255bf513ebc52faf672ca79975 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2012-05-14  Shinya Kawanaka  <shinyak@chromium.org>
>+
>+        document.execCommand('Indent') in the direct child of ShadowRoot causes a crash.
>+        https://bugs.webkit.org/show_bug.cgi?id=86341
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * editing/shadow/execcommand-indent-in-shadow-expected.txt: Added.
>+        * editing/shadow/execcommand-indent-in-shadow.html: Added.
>+
> 2012-05-13  Philippe Normand  <pnormand@igalia.com>
> 
>         Unreviewed GTK test_expectations update.
>diff --git a/LayoutTests/editing/shadow/execcommand-indent-in-shadow-expected.txt b/LayoutTests/editing/shadow/execcommand-indent-in-shadow-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..866f10048261e0bbb3744ec8aa9125416865aa51
>--- /dev/null
>+++ b/LayoutTests/editing/shadow/execcommand-indent-in-shadow-expected.txt
>@@ -0,0 +1,13 @@
>+In this test, we do execCommand('Indent') in the direct child of ShadowRoot to confirm a crash doesn't happen.
>+
>+
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>+
>+
>+
>+
>diff --git a/LayoutTests/editing/shadow/execcommand-indent-in-shadow.html b/LayoutTests/editing/shadow/execcommand-indent-in-shadow.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..1ba5bdaa1f015ce793008bb39f62f5c9fe722f3c
>--- /dev/null
>+++ b/LayoutTests/editing/shadow/execcommand-indent-in-shadow.html
>@@ -0,0 +1,32 @@
>+ <!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../fast/js/resources/js-test-pre.js"></script>
>+<script src="../../fast/dom/resources/event-sender-util.js"></script>
>+<script src="../../fast/dom/shadow/resources/polyfill.js"></script>
>+</head>
>+<body>
>+
>+<p>In this test, we do execCommand('Indent') in the direct child of ShadowRoot
>+to confirm a crash doesn't happen.</p>
>+
>+<div id="container" contenteditable></div>
>+<pre id="console"></div>
>+
>+<script>
>+var shadowRoot = new WebKitShadowRoot(container);
>+shadowRoot.innerHTML = "<span contenteditable>Kotori Otonashi</span>"
>+var span = shadowRoot.firstChild;
>+
>+mouseMoveToElem(span);
>+eventSender.mouseDown();
>+eventSender.mouseUp();
>+
>+document.execCommand('Indent');
>+
>+var successfullyParsed = true;
>+</script>
>+<script src="../../fast/js/resources/js-test-post.js"></script>
>+</body>
>+
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 86341: 141660 | 141665 | 141878