[webkit-changes] [117359] trunk/Source/WebCore

Wed, 16 May 2012 16:23:43 -0700

Title: [117359] trunk/Source/WebCore
Revision
117359
Author
rn...@webkit.org
Date
2012-05-16 16:24:34 -0700 (Wed, 16 May 2012)

Log Message

Merge nextRootInlineBox with nextLinePosition
https://bugs.webkit.org/show_bug.cgi?id=81593

Reviewed by Enrica Casucci.

Call previousRootInlineBox and nextRootInlineBox in previousLinePosition and nextLinePosition respectively
to share the code. Moved out the nullity check of startBox and extracted the renderer's node from the former
two, and added editableType to their argument lists to match the interface in both use cases.

Also moved out the code to extract root inline box using RenderedPosition from those two functions and
expanded in call sites since previousLinePosition and nextLinePosition need to return the candidate position
even when the root inline box doesn't exist. To this end, renamed previousRootInlineBox and nextRootInlineBox
to previousRootInlineBoxCandidatePosition and nextRootInlineBoxCandidatePosition respectively.

In addition, got rid of one version of nextLeafWithSameEditability that adjusted node with respect to offset
This variant did:

Node* child = node->childNode(offset);
node = child ? child->nextLeafNode() : node->lastDescendant()->nextLeafNode();

instead of:

node = node->nextLeafNode();

at the beginning of the function. Observe that the former code is logically equivalent to:

Node* child = node->childNode(offset);
node = child ? child : node->lastDescendant();
node = node->nextLeafNode();

Thus, the first two lines of this logically equivalent code is added in nextLinePosition wherein we used to
call the removed variant.

This refactoring with no behavioral change would help us resolving the bug 81490.

* editing/visible_units.cpp:
(WebCore::previousRootInlineBoxCandidatePosition): Renamed from previousRootInlineBox.
(WebCore::nextRootInlineBoxCandidatePosition): Renamed from nextRootInlineBox.
(WebCore::logicallyPreviousBox): Checks the nullity of startBox's renderer and node. Also extracts the root
inline box out of the position per the interface change.
(WebCore::logicallyNextBox): Ditto.
(WebCore::previousLinePosition): Calls previousRootInlineBoxCandidatePosition.
(WebCore::nextLinePosition): Calls nextRootInlineBoxCandidatePosition.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (117358 => 117359)


--- trunk/Source/WebCore/ChangeLog	2012-05-16 23:06:50 UTC (rev 117358)
+++ trunk/Source/WebCore/ChangeLog	2012-05-16 23:24:34 UTC (rev 117359)
@@ -1,3 +1,49 @@
+2012-05-16  Ryosuke Niwa  <rn...@webkit.org>
+
+        Merge nextRootInlineBox with nextLinePosition
+        https://bugs.webkit.org/show_bug.cgi?id=81593
+
+        Reviewed by Enrica Casucci.
+
+        Call previousRootInlineBox and nextRootInlineBox in previousLinePosition and nextLinePosition respectively
+        to share the code. Moved out the nullity check of startBox and extracted the renderer's node from the former
+        two, and added editableType to their argument lists to match the interface in both use cases.
+
+        Also moved out the code to extract root inline box using RenderedPosition from those two functions and
+        expanded in call sites since previousLinePosition and nextLinePosition need to return the candidate position
+        even when the root inline box doesn't exist. To this end, renamed previousRootInlineBox and nextRootInlineBox
+        to previousRootInlineBoxCandidatePosition and nextRootInlineBoxCandidatePosition respectively.
+
+        In addition, got rid of one version of nextLeafWithSameEditability that adjusted node with respect to offset
+        This variant did:
+
+        Node* child = node->childNode(offset);
+        node = child ? child->nextLeafNode() : node->lastDescendant()->nextLeafNode();
+
+        instead of:
+
+        node = node->nextLeafNode();
+
+        at the beginning of the function. Observe that the former code is logically equivalent to:
+
+        Node* child = node->childNode(offset);
+        node = child ? child : node->lastDescendant();
+        node = node->nextLeafNode();
+
+        Thus, the first two lines of this logically equivalent code is added in nextLinePosition wherein we used to
+        call the removed variant.
+
+        This refactoring with no behavioral change would help us resolving the bug 81490.
+
+        * editing/visible_units.cpp:
+        (WebCore::previousRootInlineBoxCandidatePosition): Renamed from previousRootInlineBox.
+        (WebCore::nextRootInlineBoxCandidatePosition): Renamed from nextRootInlineBox.
+        (WebCore::logicallyPreviousBox): Checks the nullity of startBox's renderer and node. Also extracts the root
+        inline box out of the position per the interface change.
+        (WebCore::logicallyNextBox): Ditto.
+        (WebCore::previousLinePosition): Calls previousRootInlineBoxCandidatePosition.
+        (WebCore::nextLinePosition): Calls nextRootInlineBoxCandidatePosition.
+
 2012-05-16  Noel Gordon  <noel.gor...@gmail.com>
 
         [chromium] Remove ImageDecoderCG.cpp from platform/image-decoders

Modified: trunk/Source/WebCore/editing/visible_units.cpp (117358 => 117359)


--- trunk/Source/WebCore/editing/visible_units.cpp	2012-05-16 23:06:50 UTC (rev 117358)
+++ trunk/Source/WebCore/editing/visible_units.cpp	2012-05-16 23:24:34 UTC (rev 117359)
@@ -69,20 +69,6 @@
     return 0;
 }
 
-static Node* nextLeafWithSameEditability(Node* node, int offset)
-{
-    bool editable = node->rendererIsEditable();
-    ASSERT(offset >= 0);
-    Node* child = node->childNode(offset);
-    node = child ? child->nextLeafNode() : node->lastDescendant()->nextLeafNode();
-    while (node) {
-        if (editable == node->rendererIsEditable())
-            return node;
-        node = node->nextLeafNode();
-    }
-    return 0;
-}
-
 static Node* nextLeafWithSameEditability(Node* node, EditableType editableType = ContentIsEditable)
 {
     if (!node)
@@ -99,69 +85,51 @@
 }
 
 // FIXME: consolidate with code in previousLinePosition.
-static const RootInlineBox* previousRootInlineBox(const InlineBox* box, const VisiblePosition& visiblePosition)
+static Position previousRootInlineBoxCandidatePosition(Node* node, const VisiblePosition& visiblePosition, EditableType editableType)
 {
-    Node* highestRoot = highestEditableRoot(visiblePosition.deepEquivalent(), ContentIsEditable);
-
-    if (!box->renderer() || !box->renderer()->node())
-        return 0;
-
-    Node* node = box->renderer()->node();
+    Node* highestRoot = highestEditableRoot(visiblePosition.deepEquivalent(), editableType);
     Node* enclosingBlockNode = enclosingNodeWithNonInlineRenderer(node);
-    Node* previousNode = previousLeafWithSameEditability(node, ContentIsEditable);
+    Node* previousNode = previousLeafWithSameEditability(node, editableType);
 
     while (previousNode && enclosingBlockNode == enclosingNodeWithNonInlineRenderer(previousNode))
-        previousNode = previousLeafWithSameEditability(previousNode, ContentIsEditable);
+        previousNode = previousLeafWithSameEditability(previousNode, editableType);
   
     while (previousNode && !previousNode->isShadowRoot()) {
-        if (highestEditableRoot(firstPositionInOrBeforeNode(previousNode), ContentIsEditable) != highestRoot)
+        if (highestEditableRoot(firstPositionInOrBeforeNode(previousNode), editableType) != highestRoot)
             break;
 
         Position pos = previousNode->hasTagName(brTag) ? positionBeforeNode(previousNode) :
             createLegacyEditingPosition(previousNode, caretMaxOffset(previousNode));
         
-        if (pos.isCandidate()) {
-            RenderedPosition renderedPos(pos, DOWNSTREAM);
-            RootInlineBox* root = renderedPos.rootBox();
-            if (root)
-                return root;
-        }
+        if (pos.isCandidate())
+            return pos;
 
-        previousNode = previousLeafWithSameEditability(previousNode, ContentIsEditable);
+        previousNode = previousLeafWithSameEditability(previousNode, editableType);
     }
-    return 0;
+    return Position();
 }
 
-static const RootInlineBox* nextRootInlineBox(const InlineBox* box, const VisiblePosition& visiblePosition)
+static Position nextRootInlineBoxCandidatePosition(Node* node, const VisiblePosition& visiblePosition, EditableType editableType)
 {
-    Node* highestRoot = highestEditableRoot(visiblePosition.deepEquivalent(), ContentIsEditable);
-
-    if (!box->renderer() || !box->renderer()->node())
-        return 0;
-
-    Node* node = box->renderer()->node();
+    Node* highestRoot = highestEditableRoot(visiblePosition.deepEquivalent(), editableType);
     Node* enclosingBlockNode = enclosingNodeWithNonInlineRenderer(node);
-    Node* nextNode = nextLeafWithSameEditability(node, ContentIsEditable);
+    Node* nextNode = nextLeafWithSameEditability(node, editableType);
     while (nextNode && enclosingBlockNode == enclosingNodeWithNonInlineRenderer(nextNode))
         nextNode = nextLeafWithSameEditability(nextNode, ContentIsEditable);
   
     while (nextNode && !nextNode->isShadowRoot()) {
-        if (highestEditableRoot(firstPositionInOrBeforeNode(nextNode), ContentIsEditable) != highestRoot)
+        if (highestEditableRoot(firstPositionInOrBeforeNode(nextNode), editableType) != highestRoot)
             break;
 
         Position pos;
         pos = createLegacyEditingPosition(nextNode, caretMinOffset(nextNode));
         
-        if (pos.isCandidate()) {
-            RenderedPosition renderedPos(pos, DOWNSTREAM);
-            RootInlineBox* root = renderedPos.rootBox();
-            if (root)
-                return root;
-        }
+        if (pos.isCandidate())
+            return pos;
 
-        nextNode = nextLeafWithSameEditability(nextNode, ContentIsEditable);
+        nextNode = nextLeafWithSameEditability(nextNode, editableType);
     }
-    return 0;
+    return Position();
 }
 
 class CachedLogicallyOrderedLeafBoxes {
@@ -257,8 +225,17 @@
     if (previousBox)
         return previousBox;
 
-    while (1) { 
-        const RootInlineBox* previousRoot = previousRootInlineBox(startBox, visiblePosition);
+    while (1) {
+        Node* startNode = startBox->renderer() ? startBox->renderer()->node() : 0;
+        if (!startNode)
+            break;
+
+        Position position = previousRootInlineBoxCandidatePosition(startNode, visiblePosition, ContentIsEditable);
+        if (position.isNull())
+            break;
+
+        RenderedPosition renderedPosition(position, DOWNSTREAM);
+        RootInlineBox* previousRoot = renderedPosition.rootBox();
         if (!previousRoot)
             break;
 
@@ -289,8 +266,17 @@
     if (nextBox)
         return nextBox;
 
-    while (1) { 
-        const RootInlineBox* nextRoot = nextRootInlineBox(startBox, visiblePosition);
+    while (1) {
+        Node* startNode = startBox->renderer() ? startBox->renderer()->node() : 0;
+        if (!startNode)
+            break;
+
+        Position position = nextRootInlineBoxCandidatePosition(startNode, visiblePosition, ContentIsEditable);
+        if (position.isNull())
+            break;
+
+        RenderedPosition renderedPosition(position, DOWNSTREAM);
+        RootInlineBox* nextRoot = renderedPosition.rootBox();
         if (!nextRoot)
             break;
 
@@ -951,7 +937,6 @@
 {
     Position p = visiblePosition.deepEquivalent();
     Node* node = p.deprecatedNode();
-    Node* highestRoot = highestEditableRoot(p, editableType);
 
     if (!node)
         return VisiblePosition();
@@ -975,28 +960,12 @@
     }
 
     if (!root) {
-        // This containing editable block does not have a previous line.
-        // Need to move back to previous containing editable block in this root editable
-        // block and find the last root line box in that block.
-        Node* startBlock = enclosingNodeWithNonInlineRenderer(node);
-        Node* n = previousLeafWithSameEditability(node, editableType);
-        while (n && startBlock == enclosingNodeWithNonInlineRenderer(n))
-            n = previousLeafWithSameEditability(n, editableType);
-        while (n) {
-            if (highestEditableRoot(firstPositionInOrBeforeNode(n), editableType) != highestRoot)
-                break;
-            Position pos = n->hasTagName(brTag) ? positionBeforeNode(n) : createLegacyEditingPosition(n, caretMaxOffset(n));
-            if (pos.isCandidate()) {
-                pos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
-                if (box) {
-                    // previous root line box found
-                    root = box->root();
-                    break;
-                }
-
-                return VisiblePosition(pos, DOWNSTREAM);
-            }
-            n = previousLeafWithSameEditability(n, editableType);
+        Position position = previousRootInlineBoxCandidatePosition(node, visiblePosition, editableType);
+        if (position.isNotNull()) {
+            RenderedPosition renderedPosition(position);
+            root = renderedPosition.rootBox();
+            if (!root)
+                return position;
         }
     }
     
@@ -1019,12 +988,10 @@
     return VisiblePosition(firstPositionInNode(rootElement), DOWNSTREAM);
 }
 
-
 VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int lineDirectionPoint, EditableType editableType)
 {
     Position p = visiblePosition.deepEquivalent();
     Node* node = p.deprecatedNode();
-    Node* highestRoot = highestEditableRoot(p, editableType);
 
     if (!node)
         return VisiblePosition();
@@ -1048,29 +1015,15 @@
     }
 
     if (!root) {
-        // This containing editable block does not have a next line.
-        // Need to move forward to next containing editable block in this root editable
-        // block and find the first root line box in that block.
-        Node* startBlock = enclosingNodeWithNonInlineRenderer(node);
-        Node* n = nextLeafWithSameEditability(node, p.deprecatedEditingOffset());
-        while (n && startBlock == enclosingNodeWithNonInlineRenderer(n))
-            n = nextLeafWithSameEditability(n, editableType);
-        while (n) {
-            if (highestEditableRoot(firstPositionInOrBeforeNode(n), editableType) != highestRoot)
-                break;
-            Position pos = createLegacyEditingPosition(n, caretMinOffset(n));
-            if (pos.isCandidate()) {
-                ASSERT(n->renderer());
-                pos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
-                if (box) {
-                    // next root line box found
-                    root = box->root();
-                    break;
-                }
-
-                return VisiblePosition(pos, DOWNSTREAM);
-            }
-            n = nextLeafWithSameEditability(n, editableType);
+        // FIXME: We need do the same in previousLinePosition.
+        Node* child = node->childNode(p.deprecatedEditingOffset());
+        node = child ? child : node->lastDescendant();
+        Position position = nextRootInlineBoxCandidatePosition(node, visiblePosition, editableType);
+        if (position.isNotNull()) {
+            RenderedPosition renderedPosition(position);
+            root = renderedPosition.rootBox();
+            if (!root)
+                return position;
         }
     }
     

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to