charactersAroundPosition can be wrong because it crosses editing boundaries
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Aug 2016 21:11:55 +0000 (21:11 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 26 Aug 2016 21:11:55 +0000 (21:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=161215
-and corresponding-
rdar://problem/27933564

Reviewed by Ryosuke Niwa.

Source/WebCore:

charactersAroundPosition() should not cross editing boundaries. This patch fixes
that by making nextCharacterBoundaryInDirection() take an
EditingBoundaryCrossingRule parameter to pass onto VisiblePosition::next() and
VisiblePosition::previous().

* editing/VisibleUnits.cpp:
(WebCore::nextCharacterBoundaryInDirection):
(WebCore::positionOfNextBoundaryOfGranularity):
(WebCore::charactersAroundPosition):

LayoutTests:

New test.
* editing/mac/spelling/accept-candidate-without-crossing-editing-boundary-expected.txt: Added.
* editing/mac/spelling/accept-candidate-without-crossing-editing-boundary.html: Added.

This test is going back to its pre-https://trac.webkit.org/changeset/195078 state.
That change caused this test to have a different layout because it caused more
layouts to happen. Now that we don’t allow the call to charactersAroundPosition()
to cross editing boundaries, those layouts don’t happen, and we have the old
behavior back.
* platform/mac/fast/dom/focus-contenteditable-expected.txt:

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

LayoutTests/ChangeLog
LayoutTests/editing/mac/spelling/accept-candidate-without-crossing-editing-boundary-expected.txt [new file with mode: 0644]
LayoutTests/editing/mac/spelling/accept-candidate-without-crossing-editing-boundary.html [new file with mode: 0644]
LayoutTests/platform/mac/fast/dom/focus-contenteditable-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/editing/VisibleUnits.cpp

index 7e7d73c..99b0185 100644 (file)
@@ -1,3 +1,23 @@
+2016-08-26  Beth Dakin  <bdakin@apple.com>
+
+        charactersAroundPosition can be wrong because it crosses editing boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=161215
+        -and corresponding-
+        rdar://problem/27933564
+
+        Reviewed by Ryosuke Niwa.
+
+        New test.
+        * editing/mac/spelling/accept-candidate-without-crossing-editing-boundary-expected.txt: Added.
+        * editing/mac/spelling/accept-candidate-without-crossing-editing-boundary.html: Added.
+
+        This test is going back to its pre-https://trac.webkit.org/changeset/195078 state. 
+        That change caused this test to have a different layout because it caused more 
+        layouts to happen. Now that we don’t allow the call to charactersAroundPosition() 
+        to cross editing boundaries, those layouts don’t happen, and we have the old 
+        behavior back.
+        * platform/mac/fast/dom/focus-contenteditable-expected.txt:
+
 2016-08-26  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Test and address issues in IndexedDB.requestData
diff --git a/LayoutTests/editing/mac/spelling/accept-candidate-without-crossing-editing-boundary-expected.txt b/LayoutTests/editing/mac/spelling/accept-candidate-without-crossing-editing-boundary-expected.txt
new file mode 100644 (file)
index 0000000..9eadf44
--- /dev/null
@@ -0,0 +1,28 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document toDOMRange:range from 1 of #text > DIV > DIV > BODY > HTML > #document to 1 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 1 of #text > DIV > DIV > BODY > HTML > #document to 1 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 2 of #text > DIV > DIV > BODY > HTML > #document to 2 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldInsertText:happy replacingDOMRange:range from 0 of #text > DIV > DIV > BODY > HTML > #document to 2 of #text > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionTyped
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document toDOMRange:range from 5 of #text > DIV > DIV > BODY > HTML > #document to 5 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: shouldInsertText:  replacingDOMRange:range from 5 of #text > DIV > DIV > BODY > HTML > #document to 5 of #text > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionTyped
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 5 of #text > DIV > DIV > BODY > HTML > #document to 5 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 6 of #text > DIV > DIV > BODY > HTML > #document to 6 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This test verifies that accepted candidates replace the text before the caret.
+
+happy 
+
+
diff --git a/LayoutTests/editing/mac/spelling/accept-candidate-without-crossing-editing-boundary.html b/LayoutTests/editing/mac/spelling/accept-candidate-without-crossing-editing-boundary.html
new file mode 100644 (file)
index 0000000..8c3d790
--- /dev/null
@@ -0,0 +1,38 @@
+<html>
+<head>
+<script src=../../editing.js></script>
+<script src="../../../resources/js-test-pre.js"></script>
+<script>
+
+function editingTest() {
+    edit = document.getElementById('edit');
+    edit.focus();
+    typeCharacterCommand('h');
+    typeCharacterCommand('a');
+    if (window.internals)
+        internals.handleAcceptedCandidate("happy");
+
+    if (window.testRunner)
+        testRunner.dumpAsText(true);
+}
+
+</script>
+</head>
+<body>
+<p>This test verifies that accepted candidates replace the text before the caret.</p>
+<div style="border:1px solid black;">
+    <div contenteditable="true" id="edit"></div>
+</div>
+
+<div style="visibility:hidden;">
+    <br>
+</div>
+
+<div style="width: 1px; height: 1px;"></div>
+
+<script>
+runEditingTest();
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index a274295..30de329 100644 (file)
@@ -6,7 +6,7 @@ layer at (0,0) size 785x894
       RenderBlock (anonymous) at (0,0) size 769x36
         RenderText {#text} at (0,0) size 509x18
           text run at (0,0) width 509: "This test will try to call focus() on a contenteditable div, and then a normal div. "
-        RenderBR {BR} at (0,0) size 0x0
+        RenderBR {BR} at (508,14) size 1x0
         RenderText {#text} at (0,18) size 379x18
           text run at (0,18) width 379: "The window should scroll to reveal the contenteditable div."
       RenderBlock {DIV} at (0,36) size 500x800
index f52ed65..0ba1445 100644 (file)
@@ -1,3 +1,22 @@
+2016-08-26  Beth Dakin  <bdakin@apple.com>
+
+        charactersAroundPosition can be wrong because it crosses editing boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=161215
+        -and corresponding-
+        rdar://problem/27933564
+
+        Reviewed by Ryosuke Niwa.
+
+        charactersAroundPosition() should not cross editing boundaries. This patch fixes 
+        that by making nextCharacterBoundaryInDirection() take an 
+        EditingBoundaryCrossingRule parameter to pass onto VisiblePosition::next() and 
+        VisiblePosition::previous().
+
+        * editing/VisibleUnits.cpp:
+        (WebCore::nextCharacterBoundaryInDirection):
+        (WebCore::positionOfNextBoundaryOfGranularity):
+        (WebCore::charactersAroundPosition):
+
 2016-08-26  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Test and address issues in IndexedDB.requestData
index e0dc405..e80b324 100644 (file)
@@ -1637,9 +1637,9 @@ bool withinTextUnitOfGranularity(const VisiblePosition& vp, TextGranularity gran
     return (prevBoundary < vp && vp < nextBoundary);
 }
 
-static VisiblePosition nextCharacterBoundaryInDirection(const VisiblePosition& vp, SelectionDirection direction)
+static VisiblePosition nextCharacterBoundaryInDirection(const VisiblePosition& vp, SelectionDirection direction, EditingBoundaryCrossingRule rule)
 {
-    return directionIsDownstream(direction) ? vp.next() : vp.previous();
+    return directionIsDownstream(direction) ? vp.next(rule) : vp.previous(rule);
 }
 
 static VisiblePosition nextWordBoundaryInDirection(const VisiblePosition& vp, SelectionDirection direction)
@@ -1779,7 +1779,7 @@ VisiblePosition positionOfNextBoundaryOfGranularity(const VisiblePosition& vp, T
 {
     switch (granularity) {
     case CharacterGranularity:
-        return nextCharacterBoundaryInDirection(vp, direction);
+        return nextCharacterBoundaryInDirection(vp, direction, CanCrossEditingBoundary);
     case WordGranularity:
         return nextWordBoundaryInDirection(vp, direction);
     case SentenceGranularity:
@@ -1891,14 +1891,14 @@ void charactersAroundPosition(const VisiblePosition& position, UChar32& oneAfter
     VisiblePosition startPosition = position;
     VisiblePosition endPosition = position;
 
-    VisiblePosition nextPosition = nextCharacterBoundaryInDirection(position, DirectionForward);
+    VisiblePosition nextPosition = nextCharacterBoundaryInDirection(position, DirectionForward, CannotCrossEditingBoundary);
     if (nextPosition.isNotNull())
         endPosition = nextPosition;
 
-    VisiblePosition previousPosition = nextCharacterBoundaryInDirection(position, DirectionBackward);
+    VisiblePosition previousPosition = nextCharacterBoundaryInDirection(position, DirectionBackward, CannotCrossEditingBoundary);
     if (previousPosition.isNotNull()) {
         startPosition = previousPosition;
-        previousPosition = nextCharacterBoundaryInDirection(previousPosition, DirectionBackward);
+        previousPosition = nextCharacterBoundaryInDirection(previousPosition, DirectionBackward, CannotCrossEditingBoundary);
         if (previousPosition.isNotNull())
             startPosition = previousPosition;
     }