ASSERTION FAILED: comparePositions(newEnd, newStart) >= 0 in WebCore::ApplyStyleComma...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2014 17:41:03 +0000 (17:41 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2014 17:41:03 +0000 (17:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=121791

Patch by Renata Hodovan <rhodovan.u-szeged@partner.samsung.com> on 2014-02-14
Reviewed by Darin Adler.

Source/WebCore:

If WebCore::ApplyStyleCommand::applyBlockStyle() creates a TextIterator for a range
that has an element with ReplacedElement rendering object, then a ',' is emitted in the
constructor of TextIterator. Due to this comma the end of the run range can be at the
wrong position, what makes the assertion fire. This situation can be handled the same
way in TextIterator::rangeFromLocationAndLength() as we do in case of the emitted '\n's.

Test: editing/execCommand/remove-formatting-from-iframe-in-button.html

* editing/TextIterator.cpp:
(WebCore::TextIterator::rangeFromLocationAndLength):

LayoutTests:

* editing/execCommand/remove-formatting-from-iframe-in-button-expected.txt: Added.
* editing/execCommand/remove-formatting-from-iframe-in-button.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@164104 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/execCommand/remove-formatting-from-iframe-in-button-expected.txt [new file with mode: 0644]
LayoutTests/editing/execCommand/remove-formatting-from-iframe-in-button.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/TextIterator.cpp

index 88e81be..6fe6edc 100644 (file)
@@ -1,3 +1,13 @@
+2014-02-14  Renata Hodovan  <rhodovan.u-szeged@partner.samsung.com>
+
+        ASSERTION FAILED: comparePositions(newEnd, newStart) >= 0 in WebCore::ApplyStyleCommand::updateStartEnd
+        https://bugs.webkit.org/show_bug.cgi?id=121791
+
+        Reviewed by Darin Adler.
+
+        * editing/execCommand/remove-formatting-from-iframe-in-button-expected.txt: Added.
+        * editing/execCommand/remove-formatting-from-iframe-in-button.html: Added.
+
 2014-02-14  Andrei Bucur  <abucur@adobe.com>
 
         [CSS Regions] visibility: hidden on a region should hide its content
diff --git a/LayoutTests/editing/execCommand/remove-formatting-from-iframe-in-button-expected.txt b/LayoutTests/editing/execCommand/remove-formatting-from-iframe-in-button-expected.txt
new file mode 100644 (file)
index 0000000..2d76177
--- /dev/null
@@ -0,0 +1 @@
+PASSED - this test didn't fire ASSERT, bug 121791.
diff --git a/LayoutTests/editing/execCommand/remove-formatting-from-iframe-in-button.html b/LayoutTests/editing/execCommand/remove-formatting-from-iframe-in-button.html
new file mode 100644 (file)
index 0000000..3e7c2ed
--- /dev/null
@@ -0,0 +1,19 @@
+<head>
+       <script type="text/javascript">
+               function runTest() {
+                   if (window.testRunner)
+                       window.testRunner.dumpAsText();
+
+                document.designMode="on";
+            document.execCommand("selectall");
+            document.execCommand("RemoveFormat");
+            document.execCommand("inserthtml", false, "PASSED - this test didn't fire ASSERT, bug 121791.");
+        }
+       </script>
+</head>
+<body onload="runTest()">      
+       <button>
+           <iframe/>
+       </button>
+</body>
+
index f9e1655..4b8d2b5 100644 (file)
@@ -1,3 +1,21 @@
+2014-02-14  Renata Hodovan  <rhodovan.u-szeged@partner.samsung.com>
+
+        ASSERTION FAILED: comparePositions(newEnd, newStart) >= 0 in WebCore::ApplyStyleCommand::updateStartEnd
+        https://bugs.webkit.org/show_bug.cgi?id=121791
+
+        Reviewed by Darin Adler.
+
+        If WebCore::ApplyStyleCommand::applyBlockStyle() creates a TextIterator for a range
+        that has an element with ReplacedElement rendering object, then a ',' is emitted in the
+        constructor of TextIterator. Due to this comma the end of the run range can be at the
+        wrong position, what makes the assertion fire. This situation can be handled the same
+        way in TextIterator::rangeFromLocationAndLength() as we do in case of the emitted '\n's.
+
+        Test: editing/execCommand/remove-formatting-from-iframe-in-button.html
+
+        * editing/TextIterator.cpp:
+        (WebCore::TextIterator::rangeFromLocationAndLength):
+
 2014-02-14  Andrei Bucur  <abucur@adobe.com>
 
         [CSS Regions] visibility: hidden on a region should hide its content
index f339b7b..0b8cab3 100644 (file)
@@ -2450,6 +2450,17 @@ PassRefPtr<Range> TextIterator::subrange(Range* entireRange, int characterOffset
     return characterSubrange(entireRangeIterator, characterOffset, characterCount);
 }
 
+static inline bool isInsideReplacedElement(TextIterator& iterator)
+{
+    ASSERT(!iterator.atEnd());
+    ASSERT(iterator.length() == 1);
+    Node* node = iterator.node();
+    if (!node)
+        return false;
+    auto* renderer = node->renderer();
+    return renderer && isRendererReplacedElement(renderer);
+}
+
 PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope, int rangeLocation, int rangeLength, bool forSelectionPreservation)
 {
     RefPtr<Range> resultRange = scope->document().createRange();
@@ -2483,8 +2494,8 @@ PassRefPtr<Range> TextIterator::rangeFromLocationAndLength(ContainerNode* scope,
         // in those cases that textRunRange is used.
         if (foundEnd) {
             // FIXME: This is a workaround for the fact that the end of a run is often at the wrong
-            // position for emitted '\n's.
-            if (len == 1 && it.characterAt(0) == '\n') {
+            // position for emitted '\n's or if the renderer of the current node is a replaced element.
+            if (len == 1 && (it.characterAt(0) == '\n' || isInsideReplacedElement(it))) {
                 it.advance();
                 if (!it.atEnd()) {
                     RefPtr<Range> range = it.range();