Title: [91077] trunk
Revision
91077
Author
[email protected]
Date
2011-07-15 10:23:54 -0700 (Fri, 15 Jul 2011)

Log Message

--webkit-visual-word crash on mixed editability
https://bugs.webkit.org/show_bug.cgi?id=61978

--webkit-visual-word crashes (VisiblePosition.getInlineBoxAndOffset could return null box)
https://bugs.webkit.org/show_bug.cgi?id=62814

Reviewed by Ryosuke Niwa.

Source/WebCore: 

Replace previousWordPosition/nextWordPosition with previousBoundary/nextBoundary which do
not honor editable bounary. Honor editable boundary as the last stage of leftWordPosition
and rightWordPosition.

Check previousBoundary/nextBoundary against NULL.  Have a static function to encapsulate the
usage of getInlineBoxAndOffset and check the visible position is not NULL before passing to 
getInlineBoxAndOffset. Check the box returned from getInlineBoxAndOffset is not NULL before
accessing.

Test: editing/selection/move-by-word-visually-null-box.html

* editing/visible_units.cpp:
(WebCore::positionIsInBox):
(WebCore::previousWordBreakInBoxInsideBlockWithSameDirectionality):
(WebCore::lastWordBreakInBox):
(WebCore::positionIsVisuallyOrderedInBoxInBlockWithDifferentDirectionality):
(WebCore::nextWordBreakInBoxInsideBlockWithDifferentDirectionality):
(WebCore::positionIsInsideBox):
(WebCore::leftWordPositionAcrossBoundary):
(WebCore::rightWordPositionAcrossBoundary):
(WebCore::leftWordPosition):
(WebCore::rightWordPosition):

LayoutTests: 

Add a standalone test for testing getInlineBoxAndOffset returning null box.

* editing/selection/move-by-word-visually-null-box-expected.txt: Added.
* editing/selection/move-by-word-visually-null-box.html: Added.
* editing/selection/move-by-word-visually-others-expected.txt:
* editing/selection/move-by-word-visually-others.html:

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (91076 => 91077)


--- trunk/LayoutTests/ChangeLog	2011-07-15 17:17:57 UTC (rev 91076)
+++ trunk/LayoutTests/ChangeLog	2011-07-15 17:23:54 UTC (rev 91077)
@@ -1,3 +1,20 @@
+2011-07-15  Xiaomei Ji  <[email protected]>
+
+        --webkit-visual-word crash on mixed editability
+        https://bugs.webkit.org/show_bug.cgi?id=61978
+
+        --webkit-visual-word crashes (VisiblePosition.getInlineBoxAndOffset could return null box)
+        https://bugs.webkit.org/show_bug.cgi?id=62814
+
+        Reviewed by Ryosuke Niwa.
+
+        Add a standalone test for testing getInlineBoxAndOffset returning null box.
+
+        * editing/selection/move-by-word-visually-null-box-expected.txt: Added.
+        * editing/selection/move-by-word-visually-null-box.html: Added.
+        * editing/selection/move-by-word-visually-others-expected.txt:
+        * editing/selection/move-by-word-visually-others.html:
+
 2011-07-15  Stephen White  <[email protected]>
 
         Unreviewed.  Mopping up some more expectations after r91069.

Added: trunk/LayoutTests/editing/selection/move-by-word-visually-null-box-expected.txt (0 => 91077)


--- trunk/LayoutTests/editing/selection/move-by-word-visually-null-box-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-null-box-expected.txt	2011-07-15 17:23:54 UTC (rev 91077)
@@ -0,0 +1 @@
+Crash test passed

Added: trunk/LayoutTests/editing/selection/move-by-word-visually-null-box.html (0 => 91077)


--- trunk/LayoutTests/editing/selection/move-by-word-visually-null-box.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-null-box.html	2011-07-15 17:23:54 UTC (rev 91077)
@@ -0,0 +1,24 @@
+<script src=""
+<script src=""
+<script>
+
+_onload_ = function() {
+    try {
+        runTest();
+
+        // Test NULL VisiblePosition.
+        var selection = getSelection();
+        selection.setPosition(0, 0);
+        selection.modify("move", "right", "-webkit-visual-word");
+        document.body.innerHTML = "Crash test passed";
+    } finally {
+    }
+};
+
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+</script>
+<div id="testMoveByWord">
+<output><div dir=ltr title="0|0" class="test_move_by_word">
+</div>
+<div id="console"></div>

Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-others-expected.txt (91076 => 91077)


--- trunk/LayoutTests/editing/selection/move-by-word-visually-others-expected.txt	2011-07-15 17:17:57 UTC (rev 91076)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-others-expected.txt	2011-07-15 17:23:54 UTC (rev 91077)
@@ -221,4 +221,14 @@
 "䤫䡱暘倎厘    疂崝烵     abc def"[0, 1, 2, 3, 4, 9, 10, 11, 17, 21]
 Move left by one word
 "䤫䡱暘倎厘    疂崝烵     abc def"[24, 21, 17, 11, 10, 9, 4, 3, 2, 1, 0]
+Test 44, LTR:
+Move right by one word
+"abc def "[0, 4]
+Move left by one word
+" hij opq"[8, 5, 1]
+Test 45, LTR:
+Move right by one word
+"\n00"[3]
+Move left by one word
+"\n00"[3, 1]
 

Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-others.html (91076 => 91077)


--- trunk/LayoutTests/editing/selection/move-by-word-visually-others.html	2011-07-15 17:17:57 UTC (rev 91076)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-others.html	2011-07-15 17:23:54 UTC (rev 91077)
@@ -88,6 +88,12 @@
 <!-- test words not separated by spaces -->
 <div style="white-space:pre" contenteditable dir=ltr class="test_move_by_word" title="0 1 2 3 4 9 10 11 17 21|24 21 17 11 10 9 4 3 2 1 0">人一氧喝大    笑抬的     abc def</div>
 
+<!-- mixed editability -->
+<div dir=ltr class="test_move_by_word" title="0 4|8 5 1">abc def <span contenteditable> inside span </span> hij opq</div>
+
+<div dir=ltr contenteditable></div>
+00<base dir=ltr class="test_move_by_word" title="3|3 1">
+
 </div>
 <pre id="console"></pre>
 </body>

Modified: trunk/Source/WebCore/ChangeLog (91076 => 91077)


--- trunk/Source/WebCore/ChangeLog	2011-07-15 17:17:57 UTC (rev 91076)
+++ trunk/Source/WebCore/ChangeLog	2011-07-15 17:23:54 UTC (rev 91077)
@@ -1,3 +1,36 @@
+2011-07-15  Xiaomei Ji  <[email protected]>
+
+        --webkit-visual-word crash on mixed editability
+        https://bugs.webkit.org/show_bug.cgi?id=61978
+
+        --webkit-visual-word crashes (VisiblePosition.getInlineBoxAndOffset could return null box)
+        https://bugs.webkit.org/show_bug.cgi?id=62814
+
+        Reviewed by Ryosuke Niwa.
+
+        Replace previousWordPosition/nextWordPosition with previousBoundary/nextBoundary which do
+        not honor editable bounary. Honor editable boundary as the last stage of leftWordPosition
+        and rightWordPosition.
+
+        Check previousBoundary/nextBoundary against NULL.  Have a static function to encapsulate the
+        usage of getInlineBoxAndOffset and check the visible position is not NULL before passing to 
+        getInlineBoxAndOffset. Check the box returned from getInlineBoxAndOffset is not NULL before
+        accessing.
+
+        Test: editing/selection/move-by-word-visually-null-box.html
+
+        * editing/visible_units.cpp:
+        (WebCore::positionIsInBox):
+        (WebCore::previousWordBreakInBoxInsideBlockWithSameDirectionality):
+        (WebCore::lastWordBreakInBox):
+        (WebCore::positionIsVisuallyOrderedInBoxInBlockWithDifferentDirectionality):
+        (WebCore::nextWordBreakInBoxInsideBlockWithDifferentDirectionality):
+        (WebCore::positionIsInsideBox):
+        (WebCore::leftWordPositionAcrossBoundary):
+        (WebCore::rightWordPositionAcrossBoundary):
+        (WebCore::leftWordPosition):
+        (WebCore::rightWordPosition):
+
 2011-07-15  Pratik Solanki  <[email protected]>
 
         Remove unncessary creation of connectionProperties dictionary

Modified: trunk/Source/WebCore/editing/visible_units.cpp (91076 => 91077)


--- trunk/Source/WebCore/editing/visible_units.cpp	2011-07-15 17:17:57 UTC (rev 91076)
+++ trunk/Source/WebCore/editing/visible_units.cpp	2011-07-15 17:23:54 UTC (rev 91077)
@@ -1173,6 +1173,16 @@
 
 static const int invalidOffset = -1;
 
+static bool positionIsInBox(const VisiblePosition& wordBreak, const InlineBox* box, int& offsetOfWordBreak)
+{
+    if (wordBreak.isNull())
+        return false;
+
+    InlineBox* boxOfWordBreak;
+    wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
+    return box == boxOfWordBreak;
+}
+
 static VisiblePosition previousWordBreakInBoxInsideBlockWithSameDirectionality(const InlineBox* box, const VisiblePosition& previousWordBreak, int& offsetOfWordBreak)
 {
     // In a LTR block, the word break should be on the left boundary of a word.
@@ -1201,26 +1211,21 @@
         if ((box->isLeftToRightDirection() && box->nextLeafChild())
             || (!box->isLeftToRightDirection() && box->prevLeafChild())) {
     
-            VisiblePosition positionAfterWord = nextWordPosition(wordBreak);
-            VisiblePosition positionBeforeWord = previousWordPosition(positionAfterWord);
-        
-            InlineBox* boxContainingPreviousWordBreak;
-            positionBeforeWord.getInlineBoxAndOffset(boxContainingPreviousWordBreak, offsetOfWordBreak);
-        
-            if (boxContainingPreviousWordBreak == box)
-                return positionBeforeWord;
+            VisiblePosition positionAfterWord = nextBoundary(wordBreak, nextWordPositionBoundary);
+            if (positionAfterWord.isNotNull()) {
+                VisiblePosition positionBeforeWord = previousBoundary(positionAfterWord, previousWordPositionBoundary);
+            
+                if (positionIsInBox(positionBeforeWord, box, offsetOfWordBreak))
+                    return positionBeforeWord;
+            }
         }
     }
   
-    wordBreak = previousWordPosition(wordBreak);
+    wordBreak = previousBoundary(wordBreak, previousWordPositionBoundary);
     if (previousWordBreak == wordBreak)
         return VisiblePosition();
 
-    InlineBox* boxContainingPreviousWordBreak;
-    wordBreak.getInlineBoxAndOffset(boxContainingPreviousWordBreak, offsetOfWordBreak);
-    if (boxContainingPreviousWordBreak != box)
-        return VisiblePosition();
-    return wordBreak;
+    return positionIsInBox(wordBreak, box, offsetOfWordBreak) ? wordBreak : VisiblePosition();
 }
 
 static VisiblePosition leftmostPositionInRTLBoxInLTRBlock(const InlineBox* box)
@@ -1287,25 +1292,20 @@
     if (boundaryPosition.isNull())
         return VisiblePosition();            
 
-    VisiblePosition wordBreak = nextWordPosition(boundaryPosition);
-    if (wordBreak != boundaryPosition)
-        wordBreak = previousWordPosition(wordBreak);
+    VisiblePosition wordBreak = nextBoundary(boundaryPosition, nextWordPositionBoundary);
+    if (wordBreak.isNull())
+        wordBreak = boundaryPosition;
+    else if (wordBreak != boundaryPosition)
+        wordBreak = previousBoundary(wordBreak, previousWordPositionBoundary);
 
-    InlineBox* boxOfWordBreak;
-    wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
-    if (boxOfWordBreak == box)
-        return wordBreak;
-    return VisiblePosition();    
+    return positionIsInBox(wordBreak, box, offsetOfWordBreak) ? wordBreak : VisiblePosition();    
 }
 
 static bool positionIsVisuallyOrderedInBoxInBlockWithDifferentDirectionality(const VisiblePosition& wordBreak, const InlineBox* box, int& offsetOfWordBreak)
 {
     int previousOffset = offsetOfWordBreak;
-    InlineBox* boxOfWordBreak;
-    wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
-    if (boxOfWordBreak == box && (previousOffset == invalidOffset || previousOffset < offsetOfWordBreak))
-        return true;
-    return false;
+    return positionIsInBox(wordBreak, box, offsetOfWordBreak)
+        && (previousOffset == invalidOffset || previousOffset < offsetOfWordBreak);
 }
     
 static VisiblePosition nextWordBreakInBoxInsideBlockWithDifferentDirectionality(
@@ -1321,7 +1321,7 @@
     
     bool hasSeenWordBreakInThisBox = previousWordBreak.isNotNull();
     VisiblePosition wordBreak = hasSeenWordBreakInThisBox ? previousWordBreak : Position(box->renderer()->node(), box->caretMinOffset(), Position::PositionIsOffsetInAnchor);
-    wordBreak = nextWordPosition(wordBreak);
+    wordBreak = nextBoundary(wordBreak, nextWordPositionBoundary);
   
     // Given RTL box "ABC DEF" either follows a LTR box or is the first visual box in an LTR block as an example,
     // the visual display of the RTL box is: "(0)J(10)I(9)H(8) (7)F(6)E(5)D(4) (3)C(2)B(1)A(11)",
@@ -1512,17 +1512,20 @@
     
 static bool positionIsInsideBox(const VisiblePosition& wordBreak, const InlineBox* box)
 {
-    InlineBox* boxOfWordBreak;
     int offsetOfWordBreak;
-    wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
-    return box == boxOfWordBreak && offsetOfWordBreak != box->caretMaxOffset() && offsetOfWordBreak != box->caretMinOffset();
+    return positionIsInBox(wordBreak, box, offsetOfWordBreak)
+        && offsetOfWordBreak != box->caretMaxOffset() && offsetOfWordBreak != box->caretMinOffset();
 }
 
-VisiblePosition leftWordPosition(const VisiblePosition& visiblePosition)
+static VisiblePosition leftWordPositionAcrossBoundary(const VisiblePosition& visiblePosition)
 {
     InlineBox* box;
     int offset;
     visiblePosition.getInlineBoxAndOffset(box, offset);
+
+    if (!box)
+        return VisiblePosition();
+
     TextDirection blockDirection = directionOfEnclosingBlock(visiblePosition.deepEquivalent());
     
     // FIXME: If the box's directionality is the same as that of the enclosing block, when the offset is at the box boundary
@@ -1537,9 +1540,9 @@
     VisiblePosition wordBreak;
     if (blockDirection == LTR) {
         if (box->direction() == blockDirection)
-            wordBreak = previousWordPosition(visiblePosition);
+            wordBreak = previousBoundary(visiblePosition, previousWordPositionBoundary);
         else
-            wordBreak = nextWordPosition(visiblePosition);
+            wordBreak = nextBoundary(visiblePosition, nextWordPositionBoundary);
     }
     if (wordBreak.isNotNull() && positionIsInsideBox(wordBreak, box))
         return wordBreak;
@@ -1555,11 +1558,15 @@
     return leftWordBoundary(box->prevLeafChild(), invalidOffset, blockDirection);
 }
 
-VisiblePosition rightWordPosition(const VisiblePosition& visiblePosition)
+static VisiblePosition rightWordPositionAcrossBoundary(const VisiblePosition& visiblePosition)
 {
     InlineBox* box;
     int offset;
     visiblePosition.getInlineBoxAndOffset(box, offset);
+
+    if (!box)
+        return VisiblePosition();
+
     TextDirection blockDirection = directionOfEnclosingBlock(visiblePosition.deepEquivalent());
     
     if (offset == box->caretLeftmostOffset())
@@ -1570,9 +1577,9 @@
     VisiblePosition wordBreak;
     if (blockDirection == RTL) {
         if (box->direction() == blockDirection)
-            wordBreak = previousWordPosition(visiblePosition);
+            wordBreak = previousBoundary(visiblePosition, previousWordPositionBoundary);
         else
-            wordBreak = nextWordPosition(visiblePosition);
+            wordBreak = nextBoundary(visiblePosition, nextWordPositionBoundary);
     }
     if (wordBreak.isNotNull() && positionIsInsideBox(wordBreak, box))
         return wordBreak;
@@ -1588,4 +1595,22 @@
     return rightWordBoundary(box->nextLeafChild(), invalidOffset, blockDirection);
 }
 
+VisiblePosition leftWordPosition(const VisiblePosition& visiblePosition)
+{
+    if (visiblePosition.isNull())
+        return VisiblePosition();
+
+    VisiblePosition leftWordBreak = leftWordPositionAcrossBoundary(visiblePosition);
+    return visiblePosition.honorEditableBoundaryAtOrBefore(leftWordBreak);
 }
+
+VisiblePosition rightWordPosition(const VisiblePosition& visiblePosition)
+{
+    if (visiblePosition.isNull())
+        return VisiblePosition();
+
+    VisiblePosition rightWordBreak = rightWordPositionAcrossBoundary(visiblePosition);
+    return visiblePosition.honorEditableBoundaryAtOrBefore(rightWordBreak);
+}
+
+}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to