Changeset 90072 in webkit


Ignore:
Timestamp:
Jun 29, 2011 5:10:53 PM (13 years ago)
Author:
justin.garcia@apple.com
Message:

2011-06-29 Justin Garcia <​justin.garcia@apple.com>

Reviewed by Enrica Casucci.

​https://bugs.webkit.org/show_bug.cgi?id=62922
indexForVisiblePosition(const VisiblePosition& visiblePosition) does not consider shadow content


VisiblePositions can be inside web form text regions, which use shadow trees. Made indexForVisiblePosition
aware of this, and added a new parameter to obtain the scope for a VisiblePosition, in addition to its index.


Added visiblePositionForIndex to go in the opposite direction, taking into account the scope
used to compute the index.


These two functions use TextIterators to convert between VisiblePositions and indices. But
TextIterator iteration using TextIteratorEmitsCharactersBetweenAllVisiblePositions does not
exactly match VisiblePosition iteration, so using them to preserve a selection during an
editing operation is unreliable. This can be seen in the expected results for:


editing/execCommand/indent-pre-list.html
editing/execCommand/crash-indenting-list-item.html


TextIterator's TextIteratorEmitsCharactersBetweenAllVisiblePositions mode needs to be fixed, or
these functions need to be changed to iterate using actual VisiblePositions. See:


​https://bugs.webkit.org/show_bug.cgi?id=63590
TextIterators in TextIteratorEmitsCharactersBetweenAllVisiblePositions do not exactly match VisiblePositions


Also:


​https://bugs.webkit.org/show_bug.cgi?id=63592
Use visiblePositionForIndex and indexForVisiblePosition everywhere that TextIterators are used to convert between VisiblePositions and indices


No new tests added because indexForVisiblePosition is currently only used for editing operations
that cannot be performed inside web form fields.

  • editing/ApplyBlockElementCommand.cpp: (WebCore::ApplyBlockElementCommand::doApply):
  • editing/InsertListCommand.cpp: (WebCore::InsertListCommand::doApply):
  • editing/htmlediting.cpp: (WebCore::indexForVisiblePosition): (WebCore::visiblePositionForIndex):
  • editing/htmlediting.h:
Location:
trunk/Source/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r90071 r90072  
     12011-06-29  Justin Garcia  <justin.garcia@apple.com>
     2
     3        Reviewed by Enrica Casucci.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=62922
     6        indexForVisiblePosition(const VisiblePosition& visiblePosition) does not consider shadow content
     7       
     8        VisiblePositions can be inside web form text regions, which use shadow trees. Made indexForVisiblePosition
     9        aware of this, and added a new parameter to obtain the scope for a VisiblePosition, in addition to its index.
     10       
     11        Added visiblePositionForIndex to go in the opposite direction, taking into account the scope
     12        used to compute the index.
     13       
     14        These two functions use TextIterators to convert between VisiblePositions and indices. But
     15        TextIterator iteration using TextIteratorEmitsCharactersBetweenAllVisiblePositions does not
     16        exactly match VisiblePosition iteration, so using them to preserve a selection during an
     17        editing operation is unreliable. This can be seen in the expected results for:
     18       
     19        editing/execCommand/indent-pre-list.html
     20        editing/execCommand/crash-indenting-list-item.html
     21       
     22        TextIterator's TextIteratorEmitsCharactersBetweenAllVisiblePositions mode needs to be fixed, or
     23        these functions need to be changed to iterate using actual VisiblePositions. See:
     24       
     25        https://bugs.webkit.org/show_bug.cgi?id=63590
     26        TextIterators in TextIteratorEmitsCharactersBetweenAllVisiblePositions do not exactly match VisiblePositions
     27       
     28        Also:
     29       
     30        https://bugs.webkit.org/show_bug.cgi?id=63592
     31        Use visiblePositionForIndex and indexForVisiblePosition everywhere that TextIterators are used to convert between VisiblePositions and indices
     32       
     33        No new tests added because indexForVisiblePosition is currently only used for editing operations
     34        that cannot be performed inside web form fields.
     35
     36        * editing/ApplyBlockElementCommand.cpp:
     37        (WebCore::ApplyBlockElementCommand::doApply):
     38        * editing/InsertListCommand.cpp:
     39        (WebCore::InsertListCommand::doApply):
     40        * editing/htmlediting.cpp:
     41        (WebCore::indexForVisiblePosition):
     42        (WebCore::visiblePositionForIndex):
     43        * editing/htmlediting.h:
     44
    1452011-06-29  Dimitri Glazkov  <dglazkov@chromium.org>
    246

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

    r89952 r90072  
    8282    ASSERT(!startOfSelection.isNull());
    8383    ASSERT(!endOfSelection.isNull());
    84     int startIndex = indexForVisiblePosition(startOfSelection);
    85     int endIndex = indexForVisiblePosition(endOfSelection);
     84    Element* startScope = 0;
     85    int startIndex = indexForVisiblePosition(startOfSelection, &startScope);
     86    Element* endScope = 0;
     87    int endIndex = indexForVisiblePosition(endOfSelection, &endScope);
    8688
    8789    formatSelection(startOfSelection, endOfSelection);
    8890
    8991    updateLayout();
    90 
    91     RefPtr<Range> startRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), startIndex, 0, true);
    92     RefPtr<Range> endRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), endIndex, 0, true);
    93     if (startRange && endRange)
    94         setEndingSelection(VisibleSelection(startRange->startPosition(), endRange->startPosition(), DOWNSTREAM));
     92   
     93    ASSERT(startScope == endScope);
     94    ASSERT(startIndex >= 0);
     95    ASSERT(startIndex <= endIndex);
     96    if (startScope == endScope && startIndex >= 0 && startIndex <= endIndex) {
     97        VisiblePosition start(visiblePositionForIndex(startIndex, startScope));
     98        VisiblePosition end(visiblePositionForIndex(endIndex, endScope));
     99        if (start.isNotNull() && end.isNotNull())
     100            setEndingSelection(VisibleSelection(start, end));
     101    }
    95102}
    96103

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

    r81165 r90072  
    153153                // the beginning of the document to the endOfSelection everytime this code is executed.
    154154                // But not using index is hard because there are so many ways we can lose selection inside doApplyForSingleParagraph.
    155                 int indexForEndOfSelection = indexForVisiblePosition(endOfSelection);
     155                Element* scope = 0;
     156                int indexForEndOfSelection = indexForVisiblePosition(endOfSelection, &scope);
    156157                doApplyForSingleParagraph(forceCreateList, listTag, currentSelection.get());
    157158                if (endOfSelection.isNull() || endOfSelection.isOrphan() || startOfLastParagraph.isNull() || startOfLastParagraph.isOrphan()) {
    158                     RefPtr<Range> lastSelectionRange = TextIterator::rangeFromLocationAndLength(document()->documentElement(), indexForEndOfSelection, 0, true);
    159                     // If lastSelectionRange is null, then some contents have been deleted from the document.
     159                    endOfSelection = visiblePositionForIndex(indexForEndOfSelection, scope);
     160                    // If endOfSelection is null, then some contents have been deleted from the document.
    160161                    // This should never happen and if it did, exit early immediately because we've lost the loop invariant.
    161                     ASSERT(lastSelectionRange);
    162                     if (!lastSelectionRange)
     162                    ASSERT(endOfSelection.isNotNull());
     163                    if (endOfSelection.isNull())
    163164                        return;
    164                     endOfSelection = lastSelectionRange->startPosition();
    165165                    startOfLastParagraph = startOfParagraph(endOfSelection, CanSkipOverEditingBoundary);
    166166                }

  • trunk/Source/WebCore/editing/VisiblePosition.h

    r89091 r90072  
    8080    UChar32 characterBefore() const { return previous().characterAfter(); }
    8181
     82    // FIXME: This does not handle [table, 0] correctly.
    8283    Element* rootEditableElement() const { return m_deepPosition.isNotNull() ? m_deepPosition.deprecatedNode()->rootEditableElement() : 0; }
    8384   

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

    r89952 r90072  
    10111011}
    10121012
    1013 
    1014 int indexForVisiblePosition(const VisiblePosition& visiblePosition)
     1013// FIXME: indexForVisiblePosition and visiblePositionForIndex use TextIterators to convert between
     1014// VisiblePositions and indices. But TextIterator iteration using TextIteratorEmitsCharactersBetweenAllVisiblePositions
     1015// does not exactly match VisiblePosition iteration, so using them to preserve a selection during an editing
     1016// opertion is unreliable. TextIterator's TextIteratorEmitsCharactersBetweenAllVisiblePositions mode needs to be fixed,
     1017// or these functions need to be changed to iterate using actual VisiblePositions.
     1018// FIXME: Deploy these functions everywhere that TextIterators are used to convert between VisiblePositions and indices.
     1019int indexForVisiblePosition(const VisiblePosition& visiblePosition, Element **scope)
    10151020{
    10161021    if (visiblePosition.isNull())
    10171022        return 0;
     1023       
    10181024    Position p(visiblePosition.deepEquivalent());
    1019     RefPtr<Range> range = Range::create(p.anchorNode()->document(), firstPositionInNode(p.anchorNode()->document()->documentElement()),
     1025    Document* document = p.anchorNode()->document();
     1026   
     1027    Element* root;
     1028    Node* shadowRoot = p.anchorNode()->shadowTreeRootNode();
     1029   
     1030    if (shadowRoot) {
     1031        // Use the shadow root for form elements, since TextIterators will not enter shadow content.
     1032        ASSERT(shadowRoot->isElementNode());
     1033        root = static_cast<Element *>(shadowRoot);
     1034    } else
     1035        root = document->documentElement();
     1036   
     1037    if (scope) {
     1038        ASSERT(!*scope);
     1039        *scope = root;
     1040    }
     1041   
     1042    RefPtr<Range> range = Range::create(document,
     1043                                        firstPositionInNode(root),
    10201044                                        p.parentAnchoredEquivalent());
     1045   
    10211046    return TextIterator::rangeLength(range.get(), true);
     1047}
     1048
     1049VisiblePosition visiblePositionForIndex(int index, Element *scope)
     1050{
     1051    RefPtr<Range> range = TextIterator::rangeFromLocationAndLength(scope, index, 0, true);
     1052    // indexForVisiblePosition can create a
     1053    if (!range)
     1054        return VisiblePosition();
     1055    return VisiblePosition(range->startPosition());
    10221056}
    10231057

  • trunk/Source/WebCore/editing/htmlediting.h

    r86491 r90072  
    174174   
    175175int comparePositions(const VisiblePosition&, const VisiblePosition&);
    176 int indexForVisiblePosition(const VisiblePosition&);
     176
     177int indexForVisiblePosition(const VisiblePosition&, Element **scope);
     178VisiblePosition visiblePositionForIndex(int index, Element *scope);
    177179
    178180// -------------------------------------------------------------------------
Note: See TracChangeset for help on using the changeset viewer.