Changeset 91077 in webkit


Ignore:
Timestamp:
Jul 15, 2011 10:23:54 AM (13 years ago)
Author:
xji@chromium.org
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:
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r91076 r91077  
     12011-07-15  Xiaomei Ji  <xji@chromium.org>
     2
     3        --webkit-visual-word crash on mixed editability
     4        https://bugs.webkit.org/show_bug.cgi?id=61978
     5
     6        --webkit-visual-word crashes (VisiblePosition.getInlineBoxAndOffset could return null box)
     7        https://bugs.webkit.org/show_bug.cgi?id=62814
     8
     9        Reviewed by Ryosuke Niwa.
     10
     11        Add a standalone test for testing getInlineBoxAndOffset returning null box.
     12
     13        * editing/selection/move-by-word-visually-null-box-expected.txt: Added.
     14        * editing/selection/move-by-word-visually-null-box.html: Added.
     15        * editing/selection/move-by-word-visually-others-expected.txt:
     16        * editing/selection/move-by-word-visually-others.html:
     17
    1182011-07-15  Stephen White  <senorblanco@chromium.org>
    219

  • trunk/LayoutTests/editing/selection/move-by-word-visually-others-expected.txt

    r88393 r91077  
    222222Move left by one word
    223223"䤫䡱暘倎厘    疂崝烵     abc def"[24, 21, 17, 11, 10, 9, 4, 3, 2, 1, 0]
     224Test 44, LTR:
     225Move right by one word
     226"abc def "[0, 4]
     227Move left by one word
     228" hij opq"[8, 5, 1]
     229Test 45, LTR:
     230Move right by one word
     231"\n00"[3]
     232Move left by one word
     233"\n00"[3, 1]
    224234

  • trunk/LayoutTests/editing/selection/move-by-word-visually-others.html

    r88393 r91077  
    8989<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>
    9090
     91<!-- mixed editability -->
     92<div dir=ltr class="test_move_by_word" title="0 4|8 5 1">abc def <span contenteditable> inside span </span> hij opq</div>
     93
     94<div dir=ltr contenteditable></div>
     9500<base dir=ltr class="test_move_by_word" title="3|3 1">
     96
    9197</div>
    9298<pre id="console"></pre>

  • trunk/Source/WebCore/ChangeLog

    r91075 r91077  
     12011-07-15  Xiaomei Ji  <xji@chromium.org>
     2
     3        --webkit-visual-word crash on mixed editability
     4        https://bugs.webkit.org/show_bug.cgi?id=61978
     5
     6        --webkit-visual-word crashes (VisiblePosition.getInlineBoxAndOffset could return null box)
     7        https://bugs.webkit.org/show_bug.cgi?id=62814
     8
     9        Reviewed by Ryosuke Niwa.
     10
     11        Replace previousWordPosition/nextWordPosition with previousBoundary/nextBoundary which do
     12        not honor editable bounary. Honor editable boundary as the last stage of leftWordPosition
     13        and rightWordPosition.
     14
     15        Check previousBoundary/nextBoundary against NULL.  Have a static function to encapsulate the
     16        usage of getInlineBoxAndOffset and check the visible position is not NULL before passing to
     17        getInlineBoxAndOffset. Check the box returned from getInlineBoxAndOffset is not NULL before
     18        accessing.
     19
     20        Test: editing/selection/move-by-word-visually-null-box.html
     21
     22        * editing/visible_units.cpp:
     23        (WebCore::positionIsInBox):
     24        (WebCore::previousWordBreakInBoxInsideBlockWithSameDirectionality):
     25        (WebCore::lastWordBreakInBox):
     26        (WebCore::positionIsVisuallyOrderedInBoxInBlockWithDifferentDirectionality):
     27        (WebCore::nextWordBreakInBoxInsideBlockWithDifferentDirectionality):
     28        (WebCore::positionIsInsideBox):
     29        (WebCore::leftWordPositionAcrossBoundary):
     30        (WebCore::rightWordPositionAcrossBoundary):
     31        (WebCore::leftWordPosition):
     32        (WebCore::rightWordPosition):
     33
    1342011-07-15  Pratik Solanki  <psolanki@apple.com>
    235

  • trunk/Source/WebCore/editing/visible_units.cpp

    r90643 r91077  
    11741174static const int invalidOffset = -1;
    11751175
     1176static bool positionIsInBox(const VisiblePosition& wordBreak, const InlineBox* box, int& offsetOfWordBreak)
     1177{
     1178    if (wordBreak.isNull())
     1179        return false;
     1180
     1181    InlineBox* boxOfWordBreak;
     1182    wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
     1183    return box == boxOfWordBreak;
     1184}
     1185
    11761186static VisiblePosition previousWordBreakInBoxInsideBlockWithSameDirectionality(const InlineBox* box, const VisiblePosition& previousWordBreak, int& offsetOfWordBreak)
    11771187{
    … …  
    12021212            || (!box->isLeftToRightDirection() && box->prevLeafChild())) {
    12031213   
    1204             VisiblePosition positionAfterWord = nextWordPosition(wordBreak);
    1205             VisiblePosition positionBeforeWord = previousWordPosition(positionAfterWord);
    1206        
    1207             InlineBox* boxContainingPreviousWordBreak;
    1208             positionBeforeWord.getInlineBoxAndOffset(boxContainingPreviousWordBreak, offsetOfWordBreak);
    1209        
    1210             if (boxContainingPreviousWordBreak == box)
    1211                 return positionBeforeWord;
     1214            VisiblePosition positionAfterWord = nextBoundary(wordBreak, nextWordPositionBoundary);
     1215            if (positionAfterWord.isNotNull()) {
     1216                VisiblePosition positionBeforeWord = previousBoundary(positionAfterWord, previousWordPositionBoundary);
     1217           
     1218                if (positionIsInBox(positionBeforeWord, box, offsetOfWordBreak))
     1219                    return positionBeforeWord;
     1220            }
    12121221        }
    12131222    }
    12141223 
    1215     wordBreak = previousWordPosition(wordBreak);
     1224    wordBreak = previousBoundary(wordBreak, previousWordPositionBoundary);
    12161225    if (previousWordBreak == wordBreak)
    12171226        return VisiblePosition();
    12181227
    1219     InlineBox* boxContainingPreviousWordBreak;
    1220     wordBreak.getInlineBoxAndOffset(boxContainingPreviousWordBreak, offsetOfWordBreak);
    1221     if (boxContainingPreviousWordBreak != box)
    1222         return VisiblePosition();
    1223     return wordBreak;
     1228    return positionIsInBox(wordBreak, box, offsetOfWordBreak) ? wordBreak : VisiblePosition();
    12241229}
    12251230
    … …  
    12881293        return VisiblePosition();           
    12891294
    1290     VisiblePosition wordBreak = nextWordPosition(boundaryPosition);
    1291     if (wordBreak != boundaryPosition)
    1292         wordBreak = previousWordPosition(wordBreak);
    1293 
    1294     InlineBox* boxOfWordBreak;
    1295     wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
    1296     if (boxOfWordBreak == box)
    1297         return wordBreak;
    1298     return VisiblePosition();   
     1295    VisiblePosition wordBreak = nextBoundary(boundaryPosition, nextWordPositionBoundary);
     1296    if (wordBreak.isNull())
     1297        wordBreak = boundaryPosition;
     1298    else if (wordBreak != boundaryPosition)
     1299        wordBreak = previousBoundary(wordBreak, previousWordPositionBoundary);
     1300
     1301    return positionIsInBox(wordBreak, box, offsetOfWordBreak) ? wordBreak : VisiblePosition();   
    12991302}
    13001303
    … …  
    13021305{
    13031306    int previousOffset = offsetOfWordBreak;
    1304     InlineBox* boxOfWordBreak;
    1305     wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
    1306     if (boxOfWordBreak == box && (previousOffset == invalidOffset || previousOffset < offsetOfWordBreak))
    1307         return true;
    1308     return false;
     1307    return positionIsInBox(wordBreak, box, offsetOfWordBreak)
     1308        && (previousOffset == invalidOffset || previousOffset < offsetOfWordBreak);
    13091309}
    13101310   
    … …  
    13221322    bool hasSeenWordBreakInThisBox = previousWordBreak.isNotNull();
    13231323    VisiblePosition wordBreak = hasSeenWordBreakInThisBox ? previousWordBreak : Position(box->renderer()->node(), box->caretMinOffset(), Position::PositionIsOffsetInAnchor);
    1324     wordBreak = nextWordPosition(wordBreak);
     1324    wordBreak = nextBoundary(wordBreak, nextWordPositionBoundary);
    13251325 
    13261326    // Given RTL box "ABC DEF" either follows a LTR box or is the first visual box in an LTR block as an example,
    … …  
    15131513static bool positionIsInsideBox(const VisiblePosition& wordBreak, const InlineBox* box)
    15141514{
    1515     InlineBox* boxOfWordBreak;
    15161515    int offsetOfWordBreak;
    1517     wordBreak.getInlineBoxAndOffset(boxOfWordBreak, offsetOfWordBreak);
    1518     return box == boxOfWordBreak && offsetOfWordBreak != box->caretMaxOffset() && offsetOfWordBreak != box->caretMinOffset();
    1519 }
    1520 
    1521 VisiblePosition leftWordPosition(const VisiblePosition& visiblePosition)
     1516    return positionIsInBox(wordBreak, box, offsetOfWordBreak)
     1517        && offsetOfWordBreak != box->caretMaxOffset() && offsetOfWordBreak != box->caretMinOffset();
     1518}
     1519
     1520static VisiblePosition leftWordPositionAcrossBoundary(const VisiblePosition& visiblePosition)
    15221521{
    15231522    InlineBox* box;
    15241523    int offset;
    15251524    visiblePosition.getInlineBoxAndOffset(box, offset);
     1525
     1526    if (!box)
     1527        return VisiblePosition();
     1528
    15261529    TextDirection blockDirection = directionOfEnclosingBlock(visiblePosition.deepEquivalent());
    15271530   
    … …  
    15381541    if (blockDirection == LTR) {
    15391542        if (box->direction() == blockDirection)
    1540             wordBreak = previousWordPosition(visiblePosition);
     1543            wordBreak = previousBoundary(visiblePosition, previousWordPositionBoundary);
    15411544        else
    1542             wordBreak = nextWordPosition(visiblePosition);
     1545            wordBreak = nextBoundary(visiblePosition, nextWordPositionBoundary);
    15431546    }
    15441547    if (wordBreak.isNotNull() && positionIsInsideBox(wordBreak, box))
    … …  
    15561559}
    15571560
    1558 VisiblePosition rightWordPosition(const VisiblePosition& visiblePosition)
     1561static VisiblePosition rightWordPositionAcrossBoundary(const VisiblePosition& visiblePosition)
    15591562{
    15601563    InlineBox* box;
    15611564    int offset;
    15621565    visiblePosition.getInlineBoxAndOffset(box, offset);
     1566
     1567    if (!box)
     1568        return VisiblePosition();
     1569
    15631570    TextDirection blockDirection = directionOfEnclosingBlock(visiblePosition.deepEquivalent());
    15641571   
    … …  
    15711578    if (blockDirection == RTL) {
    15721579        if (box->direction() == blockDirection)
    1573             wordBreak = previousWordPosition(visiblePosition);
     1580            wordBreak = previousBoundary(visiblePosition, previousWordPositionBoundary);
    15741581        else
    1575             wordBreak = nextWordPosition(visiblePosition);
     1582            wordBreak = nextBoundary(visiblePosition, nextWordPositionBoundary);
    15761583    }
    15771584    if (wordBreak.isNotNull() && positionIsInsideBox(wordBreak, box))
    … …  
    15891596}
    15901597
    1591 }
     1598VisiblePosition leftWordPosition(const VisiblePosition& visiblePosition)
     1599{
     1600    if (visiblePosition.isNull())
     1601        return VisiblePosition();
     1602
     1603    VisiblePosition leftWordBreak = leftWordPositionAcrossBoundary(visiblePosition);
     1604    return visiblePosition.honorEditableBoundaryAtOrBefore(leftWordBreak);
     1605}
     1606
     1607VisiblePosition rightWordPosition(const VisiblePosition& visiblePosition)
     1608{
     1609    if (visiblePosition.isNull())
     1610        return VisiblePosition();
     1611
     1612    VisiblePosition rightWordBreak = rightWordPositionAcrossBoundary(visiblePosition);
     1613    return visiblePosition.honorEditableBoundaryAtOrBefore(rightWordBreak);
     1614}
     1615
     1616}
Note: See TracChangeset for help on using the changeset viewer.