2010-09-08 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Sep 2010 19:24:10 +0000 (19:24 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 8 Sep 2010 19:24:10 +0000 (19:24 +0000)
        Reviewed by Tony Chang.

        applyInlineStyleToRange needs cleanup
        https://bugs.webkit.org/show_bug.cgi?id=45008

        Removed rangeIsEmpty and extracted the entire loop into applyInlineStyleToNodeRange.
        applyInlineStyleToRange is now a wrapper that fixes range and passes it on to applyInlineStyleToNodeRange.

        No new tests are added since this is a cleanup.

        * editing/ApplyStyleCommand.cpp:
        (WebCore::ApplyStyleCommand::applyInlineStyleToRange): Cleaned up.
        (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange): Extracted from applyInlineStyleToRange.
        * editing/ApplyStyleCommand.h:

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

WebCore/ChangeLog
WebCore/editing/ApplyStyleCommand.cpp
WebCore/editing/ApplyStyleCommand.h

index cb41387..9ebc12d 100644 (file)
@@ -1,3 +1,20 @@
+2010-09-08  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Tony Chang.
+
+        applyInlineStyleToRange needs cleanup
+        https://bugs.webkit.org/show_bug.cgi?id=45008
+
+        Removed rangeIsEmpty and extracted the entire loop into applyInlineStyleToNodeRange.
+        applyInlineStyleToRange is now a wrapper that fixes range and passes it on to applyInlineStyleToNodeRange.
+
+        No new tests are added since this is a cleanup.
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyInlineStyleToRange): Cleaned up.
+        (WebCore::ApplyStyleCommand::applyInlineStyleToNodeRange): Extracted from applyInlineStyleToRange.
+        * editing/ApplyStyleCommand.h:
+
 2010-09-08  Andy Estes  <aestes@apple.com>
 
         Rubber-stamped by Darin Adler.
index 4385a16..f9cd33a 100644 (file)
@@ -1037,7 +1037,7 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclaration *style)
             RefPtr<CSSMutableStyleDeclaration> embeddingStyle = CSSMutableStyleDeclaration::create();
             embeddingStyle->setProperty(CSSPropertyUnicodeBidi, CSSValueEmbed);
             embeddingStyle->setProperty(CSSPropertyDirection, direction);
-            applyInlineStyleToRange(embeddingStyle.get(), embeddingApplyStart, embeddingApplyEnd);
+            fixRangeAndApplyInlineStyle(embeddingStyle.get(), embeddingApplyStart, embeddingApplyEnd);
 
             if (styleWithoutEmbedding)
                 styleToApply = styleWithoutEmbedding;
@@ -1049,7 +1049,7 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclaration *style)
         }
     }
 
-    applyInlineStyleToRange(styleToApply.get(), start, end);
+    fixRangeAndApplyInlineStyle(styleToApply.get(), start, end);
 
     // Remove dummy style spans created by splitting text elements.
     cleanupUnstyledAppleStyleSpans(startDummySpanAncestor);
@@ -1057,78 +1057,71 @@ void ApplyStyleCommand::applyInlineStyle(CSSMutableStyleDeclaration *style)
         cleanupUnstyledAppleStyleSpans(endDummySpanAncestor);
 }
 
-void ApplyStyleCommand::applyInlineStyleToRange(CSSMutableStyleDeclaration* style, const Position& start, const Position& rangeEnd)
+void ApplyStyleCommand::fixRangeAndApplyInlineStyle(CSSMutableStyleDeclaration* style, const Position& start, const Position& end)
 {
-    Node* node = start.node();
-    Position end = rangeEnd;
-
-    bool rangeIsEmpty = false;
+    Node* startNode = start.node();
 
     if (start.deprecatedEditingOffset() >= caretMaxOffset(start.node())) {
-        node = node->traverseNextNode();
-        Position newStart = Position(node, 0);
-        if (!node || comparePositions(end, newStart) < 0)
-            rangeIsEmpty = true;
-    }
-
-    if (!rangeIsEmpty) {
-        // pastEndNode is the node after the last fully selected node.
-        Node* pastEndNode = end.node();
-        if (end.deprecatedEditingOffset() >= caretMaxOffset(end.node()))
-            pastEndNode = end.node()->traverseNextSibling();
-        // FIXME: Callers should perform this operation on a Range that includes the br
-        // if they want style applied to the empty line.
-        if (start == end && start.node()->hasTagName(brTag))
-            pastEndNode = start.node()->traverseNextNode();
-        // Add the style to selected inline runs.
-        for (Node* next; node && node != pastEndNode; node = next) {
-            
-            next = node->traverseNextNode();
-            
-            if (!node->renderer() || !node->isContentEditable())
-                continue;
-            
-            if (!node->isContentRichlyEditable() && node->isHTMLElement()) {
-                // This is a plaintext-only region. Only proceed if it's fully selected.
-                // pastEndNode is the node after the last fully selected node, so if it's inside node then
-                // node isn't fully selected.
-                if (pastEndNode && pastEndNode->isDescendantOf(node))
-                    break;
-                // Add to this element's inline style and skip over its contents.
-                HTMLElement* element = static_cast<HTMLElement*>(node);
-                RefPtr<CSSMutableStyleDeclaration> inlineStyle = element->getInlineStyleDecl()->copy();
-                inlineStyle->merge(style);
-                setNodeAttribute(element, styleAttr, inlineStyle->cssText());
-                next = node->traverseNextSibling();
-                continue;
-            }
+        startNode = startNode->traverseNextNode();
+        if (!startNode || comparePositions(end, Position(startNode, 0)) < 0)
+            return;
+    }
+
+    Node* pastEndNode = end.node();
+    if (end.deprecatedEditingOffset() >= caretMaxOffset(end.node()))
+        pastEndNode = end.node()->traverseNextSibling();
+
+    // FIXME: Callers should perform this operation on a Range that includes the br
+    // if they want style applied to the empty line.
+    if (start == end && start.node()->hasTagName(brTag))
+        pastEndNode = start.node()->traverseNextNode();
+
+    applyInlineStyleToNodeRange(style, startNode, pastEndNode);
+}
+
+void ApplyStyleCommand::applyInlineStyleToNodeRange(CSSMutableStyleDeclaration* style, Node* node, Node* pastEndNode)
+{
+    for (Node* next; node && node != pastEndNode; node = next) {
+        next = node->traverseNextNode();
         
-            if (isBlock(node))
-                continue;
-                
-            if (node->childNodeCount()) {
-                if (editingIgnoresContent(node)) {
-                    next = node->traverseNextSibling();
-                    continue;
-                }
-                continue;
-            }
-            
-            Node* runStart = node;
-            // Find the end of the run.
-            Node* sibling = node->nextSibling();
-            StyleChange startChange(style, Position(node, 0));
-            while (sibling && sibling != pastEndNode && !sibling->contains(pastEndNode)
-                   && (!isBlock(sibling) || sibling->hasTagName(brTag))
-                   && StyleChange(style, Position(sibling, 0)) == startChange) {
-                node = sibling;
-                sibling = node->nextSibling();
-            }
-            // Recompute next, since node has changed.
+        if (!node->renderer() || !node->isContentEditable())
+            continue;
+        
+        if (!node->isContentRichlyEditable() && node->isHTMLElement()) {
+            // This is a plaintext-only region. Only proceed if it's fully selected.
+            // pastEndNode is the node after the last fully selected node, so if it's inside node then
+            // node isn't fully selected.
+            if (pastEndNode && pastEndNode->isDescendantOf(node))
+                break;
+            // Add to this element's inline style and skip over its contents.
+            HTMLElement* element = static_cast<HTMLElement*>(node);
+            RefPtr<CSSMutableStyleDeclaration> inlineStyle = element->getInlineStyleDecl()->copy();
+            inlineStyle->merge(style);
+            setNodeAttribute(element, styleAttr, inlineStyle->cssText());
             next = node->traverseNextSibling();
-            // Apply the style to the run.
-            addInlineStyleIfNeeded(style, runStart, node, m_removeOnly ? DoNotAddStyledElement : AddStyledElement);
+            continue;
+        }
+        
+        if (isBlock(node))
+            continue;
+        
+        if (node->childNodeCount()) {
+            if (editingIgnoresContent(node))
+                next = node->traverseNextSibling();
+            continue;
+        }
+
+        Node* runEnd = node;
+        Node* sibling = node->nextSibling();
+        StyleChange startChange(style, Position(node, 0));
+        while (sibling && sibling != pastEndNode && !sibling->contains(pastEndNode)
+               && (!isBlock(sibling) || sibling->hasTagName(brTag))
+               && StyleChange(style, Position(sibling, 0)) == startChange) {
+            runEnd = sibling;
+            sibling = runEnd->nextSibling();
         }
+        next = runEnd->traverseNextSibling();
+        addInlineStyleIfNeeded(style, node, runEnd, m_removeOnly ? DoNotAddStyledElement : AddStyledElement);
     }
 }
 
index 969384a..9f297e2 100644 (file)
@@ -88,7 +88,8 @@ private:
     void applyBlockStyle(CSSMutableStyleDeclaration*);
     void applyRelativeFontStyleChange(CSSMutableStyleDeclaration*);
     void applyInlineStyle(CSSMutableStyleDeclaration*);
-    void applyInlineStyleToRange(CSSMutableStyleDeclaration*, const Position& start, const Position& end);
+    void fixRangeAndApplyInlineStyle(CSSMutableStyleDeclaration*, const Position& start, const Position& end);
+    void applyInlineStyleToNodeRange(CSSMutableStyleDeclaration*, Node* startNode, Node* pastEndNode);
     void addBlockStyle(const StyleChange&, HTMLElement*);
     void addInlineStyleIfNeeded(CSSMutableStyleDeclaration*, Node* start, Node* end, EAddStyledElement addStyledElement = AddStyledElement);
     void splitTextAtStart(const Position& start, const Position& end);