Reviewed by Hyatt

        Fix for this bug:

        <rdar://problem/3844662> REGRESSION (Mail): Style changes can affect adjacent, unselected text

        * khtml/editing/htmlediting.cpp:
        (khtml::ApplyStyleCommand::doApply): Move end position downstream to be sure we remove style from
        everything that could be affected.
        (khtml::ApplyStyleCommand::removeCSSStyle): Comma in intended function call was outside the
        braces, making it act as a comma operator, with a zero value as the right value!!! This made
        an important check always fail!!! It turns out that we do not want the constant at all, since
        that constant is only needed when checking a computed style, not an inline style as is being
        done here.
        (khtml::ApplyStyleCommand::removeStyle): Call nodeFullySelected with new interface.
        (khtml::ApplyStyleCommand::nodeFullySelected): Change interface and implementation to rely on
        RangeImpl::compareBoundaryPoints to perform the required check.
        * khtml/editing/htmlediting.h: Changed nodeFullySelected function interface.

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

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index b42a5b825a125e95f4012fcb354ff48da1c1a8e1..c96aa015ba75f065bc0abad286cf0372ad293b78 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,24 @@
+2004-10-22  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by Hyatt
+
+        Fix for this bug:
+        
+        <rdar://problem/3844662> REGRESSION (Mail): Style changes can affect adjacent, unselected text
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::ApplyStyleCommand::doApply): Move end position downstream to be sure we remove style from
+        everything that could be affected.
+        (khtml::ApplyStyleCommand::removeCSSStyle): Comma in intended function call was outside the
+        braces, making it act as a comma operator, with a zero value as the right value!!! This made
+        an important check always fail!!! It turns out that we do not want the constant at all, since
+        that constant is only needed when checking a computed style, not an inline style as is being
+        done here.
+        (khtml::ApplyStyleCommand::removeStyle): Call nodeFullySelected with new interface.
+        (khtml::ApplyStyleCommand::nodeFullySelected): Change interface and implementation to rely on
+        RangeImpl::compareBoundaryPoints to perform the required check.
+        * khtml/editing/htmlediting.h: Changed nodeFullySelected function interface.
+
 2004-10-22  Ken Kocienda  <kocienda@apple.com>
 
         Reviewed by Hyatt

diff --git a/WebCore/khtml/editing/htmlediting.cpp b/WebCore/khtml/editing/htmlediting.cpp
index 9b330d5d9e67d9d33316d1c47d1083f43d49affb..bf2b51fcdbdc4de0259fe7c20cb8971d9483488b 100644 (file)
--- a/WebCore/khtml/editing/htmlediting.cpp
+++ b/WebCore/khtml/editing/htmlediting.cpp
@@ -1054,7 +1054,7 @@ void ApplyStyleCommand::removeCSSStyle(HTMLElementImpl *elem)
 
     for (QPtrListIterator<CSSProperty> it(*(style()->values())); it.current(); ++it) {
         CSSProperty *property = it.current();
-        if (decl->getPropertyCSSValue(property->id()), DoNotUpdateLayout)
+        if (decl->getPropertyCSSValue(property->id()))
             removeCSSProperty(decl, property->id());
     }
 
@@ -1073,10 +1073,15 @@ void ApplyStyleCommand::removeCSSStyle(HTMLElementImpl *elem)
 
 void ApplyStyleCommand::removeStyle(const Position &start, const Position &end)
 {
+    ASSERT(start.isNotNull());
+    ASSERT(end.isNotNull());
+    ASSERT(start.node()->inDocument());
+    ASSERT(end.node()->inDocument());
+    
     NodeImpl *node = start.node();
-    while (1) {
+    while (node) {
         NodeImpl *next = node->traverseNextNode();
-        if (node->isHTMLElement() && nodeFullySelected(start, node)) {
+        if (node->isHTMLElement() && nodeFullySelected(node, start, end)) {
             HTMLElementImpl *elem = static_cast<HTMLElementImpl *>(node);
             if (isHTMLStyleNode(elem))
                 removeHTMLStyleNode(elem);
@@ -1089,19 +1094,13 @@ void ApplyStyleCommand::removeStyle(const Position &start, const Position &end)
     }
 }
 
-bool ApplyStyleCommand::nodeFullySelected(const Position &start, const NodeImpl *node) const
+bool ApplyStyleCommand::nodeFullySelected(NodeImpl *node, const Position &start, const Position &end) const
 {
     ASSERT(node);
 
-    if (node == start.node())
-        return start.offset() >= node->caretMaxOffset();
-
-    for (NodeImpl *child = node->lastChild(); child; child = child->lastChild()) {
-        if (child == start.node())
-            return start.offset() >= child->caretMaxOffset();
-    }
-
-    return !start.node()->isAncestor(node);
+    Position pos = Position(node, node->childNodeCount()).upstream();
+    return RangeImpl::compareBoundaryPoints(node, 0, start.node(), start.offset()) >= 0 &&
+        RangeImpl::compareBoundaryPoints(pos.node(), pos.offset(), end.node(), end.offset()) <= 0;
 }
 
 //------------------------------------------------------------------------------------------

diff --git a/WebCore/khtml/editing/htmlediting.h b/WebCore/khtml/editing/htmlediting.h
index 6be1a47ac94b0bcae1412fc93b48a3b33c2cf1c9..6fadb20548ef9eee51008797760b321492cd8456 100644 (file)
--- a/WebCore/khtml/editing/htmlediting.h
+++ b/WebCore/khtml/editing/htmlediting.h
@@ -247,7 +247,7 @@ private:
     void removeHTMLStyleNode(DOM::HTMLElementImpl *);
     void removeCSSStyle(DOM::HTMLElementImpl *);
     void removeStyle(const DOM::Position &start, const DOM::Position &end);
-    bool nodeFullySelected(const DOM::Position &, const DOM::NodeImpl *node) const;
+    bool nodeFullySelected(DOM::NodeImpl *, const DOM::Position &start, const DOM::Position &end) const;
 
     // style-application helpers
     bool splitTextAtStartIfNeeded(const DOM::Position &start, const DOM::Position &end);