Changeset 121303 in webkit

Timestamp:

Jun 26, 2012, 5:00:52 PM (14 years ago)

Author:

rniwa@webkit.org

Message:

Stop calling node() and deprecatedEditingOffset() in comparePositions
​https://bugs.webkit.org/show_bug.cgi?id=54535

Reviewed by Enrica Casucci.

Source/WebCore:

Replaced deprecatedNode() and deprecatedEditingOffset() by containerNode() and computeOffsetInContainerNode()
in comparePositions().

In addition, fixed a bunch of bugs in DeleteSelectionCommand::handleSpecialCaseBRDelete revealed by this change:

• Use node after position instead of deprecated node in determinig whether start and end positions at a br.
• Don't set m_startsAtEmptyLine true when BR is wrapped in a block element. The only reason this code had worked

was positions like (div, offset, 0) and (br, before) in <div><br> were treated differently, which we no longer do.

• editing/DeleteSelectionCommand.cpp:

(WebCore::DeleteSelectionCommand::handleSpecialCaseBRDelete):

• editing/htmlediting.cpp:

(WebCore::comparePositions):

LayoutTests:

Rebaselined existing tests. There are no user-visible changes.

• editing/inseting/font-size-clears-from-typing-style-expected.txt: No longer keeps div's around

when merging paragraphs.

• platform/mac/editing/deleting/delete-br-002-expected.txt:
• platform/mac/editing/deleting/delete-br-004-expected.txt:
• platform/mac/editing/deleting/delete-br-005-expected.txt:
• platform/mac/editing/deleting/delete-br-006-expected.txt:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/inserting/font-size-clears-from-typing-style-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/editing/deleting/delete-br-002-expected.txt (modified) (1 diff)
• LayoutTests/platform/mac/editing/deleting/delete-br-004-expected.txt (modified) (2 diffs)
• LayoutTests/platform/mac/editing/deleting/delete-br-005-expected.txt (modified) (2 diffs)
• LayoutTests/platform/mac/editing/deleting/delete-br-006-expected.txt (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/DeleteSelectionCommand.cpp (modified) (1 diff)
• Source/WebCore/editing/htmlediting.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-06-26  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Stop calling node() and deprecatedEditingOffset() in comparePositions
+        https://bugs.webkit.org/show_bug.cgi?id=54535
+
+        Reviewed by Enrica Casucci.
+
+        Rebaselined existing tests. There are no user-visible changes.
+
+        * editing/inseting/font-size-clears-from-typing-style-expected.txt: No longer keeps div's around
+        when merging paragraphs.
+        * platform/mac/editing/deleting/delete-br-002-expected.txt:
+        * platform/mac/editing/deleting/delete-br-004-expected.txt:
+        * platform/mac/editing/deleting/delete-br-005-expected.txt:
+        * platform/mac/editing/deleting/delete-br-006-expected.txt:
+
 2012-06-26  Alice Cheng  <alice_cheng@apple.com>
 

trunk/LayoutTests/editing/inserting/font-size-clears-from-typing-style-expected.txt

@@ -1 +1 @@
 Tests that we don't serialize redundant font-size in typingStyle. There should be no span or style attribute around A or B below.See https://bugs.webkit.org/show_bug.cgi?id=26279.
-| <div>
-|   id="wrapper"
-|   "B<#selection-caret>A"
+| "B<#selection-caret>A"
+| <br>

trunk/LayoutTests/platform/mac/editing/deleting/delete-br-002-expected.txt

@@ -25 +25 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 2 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of SPAN > DIV > BODY > HTML > #document to 2 of SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification

trunk/LayoutTests/platform/mac/editing/deleting/delete-br-004-expected.txt

@@ -26 +26 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 4 of SPAN > DIV > BODY > HTML > #document
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 9 of #text > SPAN > DIV > BODY > HTML > #document to 9 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
@@ -43 +44 @@
           RenderText {#text} at (14,70) size 92x28
             text run at (14,70) width 92: "years ago"
-        RenderText {#text} at (0,0) size 0x0
 caret: position 0 of child 3 {#text} of child 1 {SPAN} of child 1 {DIV} of body

trunk/LayoutTests/platform/mac/editing/deleting/delete-br-005-expected.txt

@@ -26 +26 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 4 of SPAN > DIV > BODY > HTML > #document
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 9 of #text > SPAN > DIV > BODY > HTML > #document to 9 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 2 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 9 of #text > SPAN > DIV > BODY > HTML > #document to 9 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of SPAN > DIV > BODY > HTML > #document to 2 of SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
@@ -45 +47 @@
           RenderText {#text} at (14,42) size 92x28
             text run at (14,42) width 92: "years ago"
-        RenderText {#text} at (0,0) size 0x0
 caret: position 0 of child 2 {#text} of child 1 {SPAN} of child 1 {DIV} of body

trunk/LayoutTests/platform/mac/editing/deleting/delete-br-006-expected.txt

@@ -26 +26 @@
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 4 of SPAN > DIV > BODY > HTML > #document
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 9 of #text > SPAN > DIV > BODY > HTML > #document to 9 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 3 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldDeleteDOMRange:range from 2 of SPAN > DIV > BODY > HTML > #document to 3 of SPAN > DIV > BODY > HTML > #document
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 9 of #text > SPAN > DIV > BODY > HTML > #document to 9 of #text > SPAN > DIV > BODY > HTML > #document toDOMRange:range from 2 of SPAN > DIV > BODY > HTML > #document to 2 of SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
@@ -48 +50 @@
           RenderText {#text} at (218,14) size 92x28
             text run at (218,14) width 92: "years ago"
-        RenderText {#text} at (0,0) size 0x0
 caret: position 23 of child 0 {#text} of child 1 {SPAN} of child 1 {DIV} of body

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-06-26  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Stop calling node() and deprecatedEditingOffset() in comparePositions
+        https://bugs.webkit.org/show_bug.cgi?id=54535
+
+        Reviewed by Enrica Casucci.
+
+        Replaced deprecatedNode() and deprecatedEditingOffset() by containerNode() and computeOffsetInContainerNode()
+        in comparePositions().
+        
+        In addition, fixed a bunch of bugs in DeleteSelectionCommand::handleSpecialCaseBRDelete revealed by this change:
+        - Use node after position instead of deprecated node in determinig whether start and end positions at a br.
+        - Don't set m_startsAtEmptyLine true when BR is wrapped in a block element. The only reason this code had worked
+        was positions like (div, offset, 0) and (br, before) in <div><br> were treated differently, which we no longer do.
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::handleSpecialCaseBRDelete):
+        * editing/htmlediting.cpp:
+        (WebCore::comparePositions):
+
 2012-06-26  Christophe Dumez  <christophe.dumez@intel.com>
 

trunk/Source/WebCore/editing/DeleteSelectionCommand.cpp

@@ -298 +298 @@
 bool DeleteSelectionCommand::handleSpecialCaseBRDelete()
 {
+    Node* nodeAfterUpstreamStart = m_upstreamStart.computeNodeAfterPosition();
+    Node* nodeAfterDownstreamStart = m_downstreamStart.computeNodeAfterPosition();
+    // Upstream end will appear before BR due to canonicalization
+    Node* nodeAfterUpstreamEnd = m_upstreamEnd.computeNodeAfterPosition();
+
+    if (!nodeAfterUpstreamStart || !nodeAfterDownstreamStart)
+        return false;
+
     // Check for special-case where the selection contains only a BR on a line by itself after another BR.
-    bool upstreamStartIsBR = m_upstreamStart.deprecatedNode()->hasTagName(brTag);
-    bool downstreamStartIsBR = m_downstreamStart.deprecatedNode()->hasTagName(brTag);
-    bool isBROnLineByItself = upstreamStartIsBR && downstreamStartIsBR && m_downstreamStart.deprecatedNode() == m_upstreamEnd.deprecatedNode();
+    bool upstreamStartIsBR = nodeAfterUpstreamStart->hasTagName(brTag);
+    bool downstreamStartIsBR = nodeAfterDownstreamStart->hasTagName(brTag);
+    bool isBROnLineByItself = upstreamStartIsBR && downstreamStartIsBR && nodeAfterDownstreamStart == nodeAfterUpstreamEnd;
     if (isBROnLineByItself) {
-        removeNode(m_downstreamStart.deprecatedNode());
+        removeNode(nodeAfterDownstreamStart);
         return true;
     }
 
-    // Not a special-case delete per se, but we can detect that the merging of content between blocks
-    // should not be done.
-    if (upstreamStartIsBR && downstreamStartIsBR) {
+    // FIXME: This code doesn't belong in here.
+    // We detect the case where the start is an empty line consisting of BR not wrapped in a block element.
+    if (upstreamStartIsBR && downstreamStartIsBR && !(isStartOfBlock(positionBeforeNode(nodeAfterUpstreamStart)) && isEndOfBlock(positionAfterNode(nodeAfterUpstreamStart)))) {
         m_startsAtEmptyLine = true;
         m_endingPosition = m_downstreamEnd;

trunk/Source/WebCore/editing/htmlediting.cpp

@@ -79 +79 @@
         return 0;
 
-    Node* nodeA = commonScope->ancestorInThisScope(a.deprecatedNode());
+    Node* nodeA = commonScope->ancestorInThisScope(a.containerNode());
     ASSERT(nodeA);
-    bool hasDescendentA = nodeA != a.deprecatedNode();
-    int offsetA = hasDescendentA ? 0 : a.deprecatedEditingOffset();
-
-    Node* nodeB = commonScope->ancestorInThisScope(b.deprecatedNode());
+    bool hasDescendentA = nodeA != a.containerNode();
+    int offsetA = hasDescendentA ? 0 : a.computeOffsetInContainerNode();
+
+    Node* nodeB = commonScope->ancestorInThisScope(b.containerNode());
     ASSERT(nodeB);
-    bool hasDescendentB = nodeB != b.deprecatedNode();
-    int offsetB = hasDescendentB ? 0 : b.deprecatedEditingOffset();
+    bool hasDescendentB = nodeB != b.containerNode();
+    int offsetB = hasDescendentB ? 0 : b.computeOffsetInContainerNode();
 
     int bias = 0;