Reviewed by Chris
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Feb 2005 22:25:28 +0000 (22:25 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 10 Feb 2005 22:25:28 +0000 (22:25 +0000)
        Fix for this bug:

        <rdar://problem/3965158> Drag-n-drop within a rich text message sometimes changes the color of the dragged text

        This change fixes the bug....and much more. Now, for the first time, the paste code can do "smart merging"
        or "smooshing" of styles during its operation. Since this new code is actively, rather than passively
        working with styles, it fixes the bug, and lays the groundwork for similar work we need to do to
        preserve quote levels in Mail.

        * khtml/css/css_valueimpl.cpp:
        (DOM::CSSMutableStyleDeclarationImpl::clear): New method.
        (DOM::CSSMutableStyleDeclarationImpl::removeBlockProperties): Ditto.
        (DOM::CSSMutableStyleDeclarationImpl::removePropertiesInSet): Ditto.
        (DOM::operator==): Add operator for CSSProperty.
        * khtml/css/css_valueimpl.h: Declare new functions.
        * khtml/editing/htmlediting.cpp:
        (khtml::isEmptyStyleSpan): Improved the test in this function, rolling together the old implementation
        with some code that did this work inline elsewhere. Sum of the parts is better than either test was by itself.
        (khtml::isStyleSpan): Check for ID_SPAN.
        (khtml::ApplyStyleCommand::removeCSSStyle): Call isEmptyStyleSpan. This was the place with an inline implementation before.
        (khtml::ReplacementFragment::ReplacementFragment): Now takes a DocumentImpl argument. No longer does a "default style"
        check, but rather calls functions which do a similar check to that, and much more.
        (khtml::ReplacementFragment::~ReplacementFragment): Deref document, and computed styles.
        (khtml::ReplacementFragment::styleForNode): New helper. Looks up and returns computed style for a node.
        (khtml::ReplacementFragment::removeNodePreservingChildren): New helper.
        (khtml::ReplacementFragment::computeStylesForNodes): New function which computes the "desired" style for
        every node in the fragment. This information is used later after paste is done as a reference for testing
        what styles need to be added, and which can be removed as redundant, from all the nodes inserted by the
        replacement code.
        (khtml::ReplacementFragment::removeStyleNodes): Clears out all style nodes from the fragment. They are
        no longer needed after the call to computeStylesForNodes(),
        (khtml::ReplaceSelectionCommand::ReplaceSelectionCommand): Add a document to the call to initialize the
        command's ReplacementFragment.
        (khtml::ReplaceSelectionCommand::doApply): Call applyStyleToInsertedNodes() after inserting nodes to make
        styles come out right.
        (khtml::ReplaceSelectionCommand::applyStyleToInsertedNodes): This is the "style smooshing" function. It
        computes the styles that need to be added to each node inserted, comparing the style it gets from just
        being inserted into its correct destination with the computed "desired style" done in the
        ReplacementFragment constructor. It then adds in all the necessary styles, and will also remove redundant styles.
        * khtml/editing/htmlediting.h: Update declarations and member variables as needed.
        * khtml/editing/markup.cpp:
        (khtml::startMarkup): Add additional style annotations to the markup we generate, so that paste code can preserve it.
        (khtml::markup): Ditto.
        (khtml::createMarkup): Ditto.

        These test results are subtly better with this change. They no longer have an unneeded empty span.
        Visually the same as before.

        * layout-tests/editing/style/remove-underline-across-paragraph-expected.txt
        * layout-tests/editing/style/remove-underline-across-paragraph-in-bold-expected.txt
        * layout-tests/editing/style/remove-underline-expected.txt
        * layout-tests/editing/style/remove-underline-from-stylesheet-expected.txt

        New tests:

        * layout-tests/editing/style/smoosh-styles-001-expected.txt
        * layout-tests/editing/style/smoosh-styles-002-expected.txt
        * layout-tests/editing/style/smoosh-styles-001.html
        * layout-tests/editing/style/smoosh-styles-002.html

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

14 files changed:
LayoutTests/editing/style/remove-underline-across-paragraph-expected.txt
LayoutTests/editing/style/remove-underline-across-paragraph-in-bold-expected.txt
LayoutTests/editing/style/remove-underline-expected.txt
LayoutTests/editing/style/remove-underline-from-stylesheet-expected.txt
LayoutTests/editing/style/smoosh-styles-001-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/smoosh-styles-001.html [new file with mode: 0644]
LayoutTests/editing/style/smoosh-styles-002-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/smoosh-styles-002.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/css/css_valueimpl.cpp
WebCore/khtml/css/css_valueimpl.h
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/htmlediting.h
WebCore/khtml/editing/markup.cpp

index 2c4e53fe9a8e38435a2bd92aad0268024a2d577a..86bdd38d68d14893f6a2dc4dfb2f6a6cfd228c65 100644 (file)
@@ -10,23 +10,20 @@ layer at (0,0) size 800x600
           RenderInline {SPAN} at (0,0) size 78x28
             RenderText {TEXT} at (0,0) size 78x28
               text run at (0,0) width 78: "xxxxxx "
-          RenderInline {SPAN} at (0,0) size 72x28
-            RenderText {TEXT} at (78,0) size 72x28
-              text run at (78,0) width 72: "xxxxxx"
-        RenderBlock (anonymous) at (14,42) size 756x28
-          RenderBlock {P} at (0,0) size 756x28
-            RenderInline {SPAN} at (0,0) size 78x28
-              RenderText {TEXT} at (0,0) size 78x28
-                text run at (0,0) width 78: " xxxxxx"
-            RenderInline {SPAN} at (0,0) size 78x28
-              RenderText {TEXT} at (78,0) size 78x28
-                text run at (78,0) width 78: " xxxxxx"
+          RenderText {TEXT} at (78,0) size 72x28
+            text run at (78,0) width 72: "xxxxxx"
+        RenderBlock {P} at (14,42) size 756x28
+          RenderText {TEXT} at (0,0) size 78x28
+            text run at (0,0) width 78: " xxxxxx"
+          RenderInline {SPAN} at (0,0) size 78x28
+            RenderText {TEXT} at (78,0) size 78x28
+              text run at (78,0) width 78: " xxxxxx"
+        RenderBlock (anonymous) at (14,70) size 756x0
         RenderBlock (anonymous) at (14,70) size 756x0
-          RenderInline {SPAN} at (0,0) size 0x0
 selection is RANGE:
-start:      position 0 of child 1 {TEXT} of child 5 {SPAN} of root {DIV}
+start:      position 0 of child 5 {TEXT} of root {DIV}
 upstream:   position 7 of child 1 {TEXT} of child 4 {SPAN} of root {DIV}
-downstream: position 0 of child 1 {TEXT} of child 5 {SPAN} of root {DIV}
-end:        position 7 of child 1 {TEXT} of child 1 {SPAN} of child 2 {P} of child 5 {SPAN} of root {DIV}
-upstream:   position 7 of child 1 {TEXT} of child 1 {SPAN} of child 2 {P} of child 5 {SPAN} of root {DIV}
-downstream: position 0 of child 1 {TEXT} of child 2 {SPAN} of child 2 {P} of child 5 {SPAN} of root {DIV}
+downstream: position 0 of child 5 {TEXT} of root {DIV}
+end:        position 7 of child 1 {TEXT} of child 6 {P} of root {DIV}
+upstream:   position 7 of child 1 {TEXT} of child 6 {P} of root {DIV}
+downstream: position 0 of child 1 {TEXT} of child 2 {SPAN} of child 6 {P} of root {DIV}
index 8b4e48a502b1642d03a7f8c48153bbfc0aeb9f7f..cb5eaba314246015d7cad96c77f2145970e778b0 100644 (file)
@@ -16,9 +16,8 @@ layer at (0,0) size 800x600
                 text run at (78,0) width 72: "xxxxxx"
         RenderBlock (anonymous) at (14,42) size 756x28
           RenderBlock {P} at (0,0) size 756x28
-            RenderInline {SPAN} at (0,0) size 78x28
-              RenderText {TEXT} at (0,0) size 78x28
-                text run at (0,0) width 78: " xxxxxx"
+            RenderText {TEXT} at (0,0) size 78x28
+              text run at (0,0) width 78: " xxxxxx"
             RenderInline {SPAN} at (0,0) size 78x28
               RenderText {TEXT} at (78,0) size 78x28
                 text run at (78,0) width 78: " xxxxxx"
@@ -29,6 +28,6 @@ selection is RANGE:
 start:      position 0 of child 1 {TEXT} of child 2 {B} of child 4 {SPAN} of root {DIV}
 upstream:   position 7 of child 1 {TEXT} of child 1 {B} of child 4 {SPAN} of root {DIV}
 downstream: position 0 of child 1 {TEXT} of child 2 {B} of child 4 {SPAN} of root {DIV}
-end:        position 7 of child 1 {TEXT} of child 1 {SPAN} of child 2 {P} of child 2 {B} of child 4 {SPAN} of root {DIV}
-upstream:   position 7 of child 1 {TEXT} of child 1 {SPAN} of child 2 {P} of child 2 {B} of child 4 {SPAN} of root {DIV}
+end:        position 7 of child 1 {TEXT} of child 2 {P} of child 2 {B} of child 4 {SPAN} of root {DIV}
+upstream:   position 7 of child 1 {TEXT} of child 2 {P} of child 2 {B} of child 4 {SPAN} of root {DIV}
 downstream: position 0 of child 1 {TEXT} of child 2 {SPAN} of child 2 {P} of child 2 {B} of child 4 {SPAN} of root {DIV}
index c44b44c71a79455763e27ffc7606ab47c7642659..c44f6371d0f5bddabfd3801265b9cb420da7620e 100644 (file)
@@ -9,16 +9,15 @@ layer at (0,0) size 800x600
         RenderInline {SPAN} at (0,0) size 78x28
           RenderText {TEXT} at (14,14) size 78x28
             text run at (14,14) width 78: "xxxxxx "
-        RenderInline {SPAN} at (0,0) size 72x28
-          RenderText {TEXT} at (92,14) size 72x28
-            text run at (92,14) width 72: "xxxxxx"
+        RenderText {TEXT} at (92,14) size 72x28
+          text run at (92,14) width 72: "xxxxxx"
         RenderInline {SPAN} at (0,0) size 78x28
           RenderText {TEXT} at (164,14) size 78x28
             text run at (164,14) width 78: " xxxxxx"
 selection is RANGE:
-start:      position 0 of child 1 {TEXT} of child 5 {SPAN} of root {DIV}
+start:      position 0 of child 5 {TEXT} of root {DIV}
 upstream:   position 7 of child 1 {TEXT} of child 4 {SPAN} of root {DIV}
-downstream: position 0 of child 1 {TEXT} of child 5 {SPAN} of root {DIV}
-end:        position 6 of child 1 {TEXT} of child 5 {SPAN} of root {DIV}
-upstream:   position 6 of child 1 {TEXT} of child 5 {SPAN} of root {DIV}
+downstream: position 0 of child 5 {TEXT} of root {DIV}
+end:        position 6 of child 5 {TEXT} of root {DIV}
+upstream:   position 6 of child 5 {TEXT} of root {DIV}
 downstream: position 0 of child 1 {TEXT} of child 6 {SPAN} of root {DIV}
index eb221d4d229feee911bda12657e2ba08ddfb4a6b..416551ecf390f3d13b6e1a333b0f62188a5bdf70 100644 (file)
@@ -10,16 +10,15 @@ layer at (0,0) size 800x600
           RenderText {TEXT} at (0,0) size 0x0
           RenderText {TEXT} at (14,14) size 78x28
             text run at (14,14) width 78: "xxxxxx "
-        RenderInline {SPAN} at (0,0) size 72x28
-          RenderText {TEXT} at (92,14) size 72x28
-            text run at (92,14) width 72: "xxxxxx"
+        RenderText {TEXT} at (92,14) size 72x28
+          text run at (92,14) width 72: "xxxxxx"
         RenderInline {SPAN} at (0,0) size 78x28
           RenderText {TEXT} at (164,14) size 78x28
             text run at (164,14) width 78: " xxxxxx"
 selection is RANGE:
-start:      position 0 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
+start:      position 0 of child 2 {TEXT} of root {DIV}
 upstream:   position 7 of child 4 {TEXT} of child 1 {SPAN} of root {DIV}
-downstream: position 0 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
-end:        position 6 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
-upstream:   position 6 of child 1 {TEXT} of child 2 {SPAN} of root {DIV}
+downstream: position 0 of child 2 {TEXT} of root {DIV}
+end:        position 6 of child 2 {TEXT} of root {DIV}
+upstream:   position 6 of child 2 {TEXT} of root {DIV}
 downstream: position 0 of child 1 {TEXT} of child 3 {SPAN} of root {DIV}
diff --git a/LayoutTests/editing/style/smoosh-styles-001-expected.txt b/LayoutTests/editing/style/smoosh-styles-001-expected.txt
new file mode 100644 (file)
index 0000000..ca773c6
--- /dev/null
@@ -0,0 +1,42 @@
+layer at (0,0) size 800x600
+  RenderCanvas at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {DIV} at (0,0) size 784x184 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x84
+          RenderText {TEXT} at (0,0) size 67x28
+            text run at (0,0) width 67: "Tests: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 727x56
+            text run at (0,28) width 727: "Pasting black (document default color) text into a block of text with a non-"
+            text run at (0,56) width 130: "default color."
+        RenderBlock {DIV} at (14,114) size 756x56
+          RenderText {TEXT} at (0,0) size 189x28
+            text run at (0,0) width 189: "Expected Results: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 442x28
+            text run at (0,28) width 442: "Should see this content in the red box below: "
+          RenderInline {SPAN} at (0,0) size 77x28 [color=#FF0000]
+            RenderText {TEXT} at (442,28) size 23x28
+              text run at (442,28) width 23: "ab"
+            RenderInline {SPAN} at (0,0) size 34x28 [color=#000000]
+              RenderText {TEXT} at (465,28) size 34x28
+                text run at (465,28) width 34: "cde"
+            RenderText {TEXT} at (499,28) size 20x28
+              text run at (499,28) width 20: "fg"
+          RenderText {TEXT} at (0,0) size 0x0
+      RenderBlock {DIV} at (0,208) size 784x32
+        RenderBlock {DIV} at (0,0) size 784x32 [border: (2px solid #FF0000)]
+          RenderInline {SPAN} at (0,0) size 77x28 [color=#FF0000]
+            RenderText {TEXT} at (2,2) size 23x28
+              text run at (2,2) width 23: "ab"
+            RenderInline {SPAN} at (0,0) size 34x28 [color=#000000]
+              RenderText {TEXT} at (25,2) size 34x28
+                text run at (25,2) width 34: "cde"
+            RenderText {TEXT} at (59,2) size 20x28
+              text run at (59,2) width 20: "fg"
+selection is CARET:
+start:      position 3 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
+upstream:   position 3 of child 1 {TEXT} of child 2 {SPAN} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 3 {TEXT} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/style/smoosh-styles-001.html b/LayoutTests/editing/style/smoosh-styles-001.html
new file mode 100644 (file)
index 0000000..d89fa11
--- /dev/null
@@ -0,0 +1,67 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    moveSelectionForwardByLineCommand();
+    extendSelectionForwardByLineCommand();
+    cutCommand();
+    deleteCommand();
+    moveSelectionBackwardByLineCommand();
+    moveSelectionForwardByCharacterCommand();
+    moveSelectionForwardByCharacterCommand();
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+
+<div class="explanation">
+<div class="scenario">
+Tests: 
+<br>
+Pasting black (document default color) text into a block of text with a non-default color.
+</div>
+<div class="expected-results">
+Expected Results:
+<br>
+Should see this content in the red box below: <span style="color: red">ab<span style="color: black">cde</span>fg</span>
+</div>
+</div>
+
+<div contenteditable id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">
+<span style="color: red">abfg</span>
+</div>
+<div class="editing">
+cde
+</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/style/smoosh-styles-002-expected.txt b/LayoutTests/editing/style/smoosh-styles-002-expected.txt
new file mode 100644 (file)
index 0000000..cfb1a37
--- /dev/null
@@ -0,0 +1,44 @@
+layer at (0,0) size 800x600
+  RenderCanvas at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x571
+      RenderBlock {DIV} at (0,0) size 784x165 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x56
+          RenderText {TEXT} at (0,0) size 67x28
+            text run at (0,0) width 67: "Tests: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,28) size 746x28
+            text run at (0,28) width 746: "Pasting text which gets its style from an <H1> tag into a block of styled text."
+        RenderBlock {DIV} at (14,86) size 756x65
+          RenderText {TEXT} at (0,0) size 189x28
+            text run at (0,0) width 189: "Expected Results: "
+          RenderBR {BR} at (0,0) size 0x0
+          RenderText {TEXT} at (0,35) size 442x28
+            text run at (0,35) width 442: "Should see this content in the red box below: "
+          RenderInline {SPAN} at (0,0) size 99x28 [color=#FF0000]
+            RenderText {TEXT} at (442,34) size 29x28
+              text run at (442,34) width 29: "ab"
+            RenderInline {SPAN} at (0,0) size 46x37 [color=#000000]
+              RenderText {TEXT} at (471,28) size 46x37
+                text run at (471,28) width 46: "cde"
+            RenderText {TEXT} at (517,34) size 24x28
+              text run at (517,34) width 24: "fg"
+          RenderText {TEXT} at (0,0) size 0x0
+      RenderBlock {DIV} at (0,189) size 784x41
+        RenderBlock {DIV} at (0,0) size 784x41 [border: (2px solid #FF0000)]
+          RenderInline {SPAN} at (0,0) size 99x28 [color=#FF0000]
+            RenderText {TEXT} at (2,8) size 29x28
+              text run at (2,8) width 29: "ab"
+            RenderInline {SPAN} at (0,0) size 46x37 [color=#000000]
+              RenderInline {B} at (0,0) size 46x37
+                RenderText {TEXT} at (31,2) size 46x37
+                  text run at (31,2) width 46: "cde"
+            RenderText {TEXT} at (77,8) size 24x28
+              text run at (77,8) width 24: "fg"
+          RenderText {TEXT} at (0,0) size 0x0
+        RenderBlock {H1} at (0,62) size 784x0
+selection is CARET:
+start:      position 3 of child 1 {TEXT} of child 1 {B} of child 2 {SPAN} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
+upstream:   position 3 of child 1 {TEXT} of child 1 {B} of child 2 {SPAN} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
+downstream: position 0 of child 3 {TEXT} of child 2 {SPAN} of child 1 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/style/smoosh-styles-002.html b/LayoutTests/editing/style/smoosh-styles-002.html
new file mode 100644 (file)
index 0000000..0d8f523
--- /dev/null
@@ -0,0 +1,67 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    moveSelectionForwardByLineCommand();
+    extendSelectionForwardByLineCommand();
+    cutCommand();
+    deleteCommand();
+    moveSelectionBackwardByLineCommand();
+    moveSelectionForwardByCharacterCommand();
+    moveSelectionForwardByCharacterCommand();
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+
+<div class="explanation">
+<div class="scenario">
+Tests: 
+<br>
+Pasting text which gets its style from an &lt;H1&gt; tag into a block of styled text.
+</div>
+<div class="expected-results">
+Expected Results:
+<br>
+Should see this content in the red box below: <span style="font-family: Lucida Grande; color: red">ab<span style="color: black; font-family: serif; font-size: 32px; font-weight: bold">cde</span>fg</span>
+</div>
+</div>
+
+<div contenteditable id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">
+<span style="font-family: Lucida Grande; color: red">abfg</span>
+</div>
+<h1>
+cde
+</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 6f9f66738fb994335273d5ab1afd63020ac70fed..38a22f8f01e2f32b7d57f92e8f68ff32f98fecc5 100644 (file)
@@ -1,3 +1,67 @@
+2005-02-10  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by Chris
+
+        Fix for this bug:
+        
+        <rdar://problem/3965158> Drag-n-drop within a rich text message sometimes changes the color of the dragged text
+        
+        This change fixes the bug....and much more. Now, for the first time, the paste code can do "smart merging"
+        or "smooshing" of styles during its operation. Since this new code is actively, rather than passively
+        working with styles, it fixes the bug, and lays the groundwork for similar work we need to do to
+        preserve quote levels in Mail.
+
+        * khtml/css/css_valueimpl.cpp:
+        (DOM::CSSMutableStyleDeclarationImpl::clear): New method.
+        (DOM::CSSMutableStyleDeclarationImpl::removeBlockProperties): Ditto.
+        (DOM::CSSMutableStyleDeclarationImpl::removePropertiesInSet): Ditto.
+        (DOM::operator==): Add operator for CSSProperty.
+        * khtml/css/css_valueimpl.h: Declare new functions.
+        * khtml/editing/htmlediting.cpp:
+        (khtml::isEmptyStyleSpan): Improved the test in this function, rolling together the old implementation
+        with some code that did this work inline elsewhere. Sum of the parts is better than either test was by itself.
+        (khtml::isStyleSpan): Check for ID_SPAN.
+        (khtml::ApplyStyleCommand::removeCSSStyle): Call isEmptyStyleSpan. This was the place with an inline implementation before.
+        (khtml::ReplacementFragment::ReplacementFragment): Now takes a DocumentImpl argument. No longer does a "default style"
+        check, but rather calls functions which do a similar check to that, and much more.
+        (khtml::ReplacementFragment::~ReplacementFragment): Deref document, and computed styles.
+        (khtml::ReplacementFragment::styleForNode): New helper. Looks up and returns computed style for a node.
+        (khtml::ReplacementFragment::removeNodePreservingChildren): New helper.
+        (khtml::ReplacementFragment::computeStylesForNodes): New function which computes the "desired" style for
+        every node in the fragment. This information is used later after paste is done as a reference for testing
+        what styles need to be added, and which can be removed as redundant, from all the nodes inserted by the
+        replacement code.
+        (khtml::ReplacementFragment::removeStyleNodes): Clears out all style nodes from the fragment. They are
+        no longer needed after the call to computeStylesForNodes(),
+        (khtml::ReplaceSelectionCommand::ReplaceSelectionCommand): Add a document to the call to initialize the
+        command's ReplacementFragment.
+        (khtml::ReplaceSelectionCommand::doApply): Call applyStyleToInsertedNodes() after inserting nodes to make
+        styles come out right.
+        (khtml::ReplaceSelectionCommand::applyStyleToInsertedNodes): This is the "style smooshing" function. It 
+        computes the styles that need to be added to each node inserted, comparing the style it gets from just
+        being inserted into its correct destination with the computed "desired style" done in the 
+        ReplacementFragment constructor. It then adds in all the necessary styles, and will also remove redundant styles.
+        * khtml/editing/htmlediting.h: Update declarations and member variables as needed.
+        * khtml/editing/markup.cpp:
+        (khtml::startMarkup): Add additional style annotations to the markup we generate, so that paste code can preserve it.
+        (khtml::markup): Ditto.
+        (khtml::createMarkup): Ditto.
+        
+        These test results are subtly better with this change. They no longer have an unneeded empty span.
+        Visually the same as before.
+        
+        * layout-tests/editing/style/remove-underline-across-paragraph-expected.txt
+        * layout-tests/editing/style/remove-underline-across-paragraph-in-bold-expected.txt
+        * layout-tests/editing/style/remove-underline-expected.txt
+        * layout-tests/editing/style/remove-underline-from-stylesheet-expected.txt
+
+        New tests:
+
+        * layout-tests/editing/style/smoosh-styles-001-expected.txt
+        * layout-tests/editing/style/smoosh-styles-002-expected.txt
+        * layout-tests/editing/style/smoosh-styles-001.html
+        * layout-tests/editing/style/smoosh-styles-002.html
+
 2005-02-10  Darin Adler  <darin@apple.com>
 
         Reviewed by Adele.
index eddb8b37b14047f0d59a8521209d5af5f8cb008c..306bff684dfc668dae5dcd78d4e70343b3de513c 100644 (file)
@@ -264,6 +264,12 @@ DOMString CSSMutableStyleDeclarationImpl::removeProperty(int propertyID, bool no
     return value;
 }
 
+void CSSMutableStyleDeclarationImpl::clear()
+{
+    m_values.clear();
+    setChanged();
+}
+
 void CSSMutableStyleDeclarationImpl::setChanged()
 {
     if (m_node) {
@@ -473,6 +479,11 @@ CSSMutableStyleDeclarationImpl *CSSMutableStyleDeclarationImpl::copyBlockPropert
     return copyPropertiesInSet(BlockProperties, sizeof(BlockProperties) / sizeof(BlockProperties[0]));
 }
 
+void CSSMutableStyleDeclarationImpl::removeBlockProperties()
+{
+    removePropertiesInSet(BlockProperties, sizeof(BlockProperties) / sizeof(BlockProperties[0]));
+}
+
 CSSMutableStyleDeclarationImpl *CSSStyleDeclarationImpl::copyPropertiesInSet(const int *set, unsigned length) const
 {
     QValueList<CSSProperty> list;
@@ -484,6 +495,15 @@ CSSMutableStyleDeclarationImpl *CSSStyleDeclarationImpl::copyPropertiesInSet(con
     return new CSSMutableStyleDeclarationImpl(0, list);
 }
 
+void CSSMutableStyleDeclarationImpl::removePropertiesInSet(const int *set, unsigned length)
+{
+    for (unsigned i = 0; i < length; i++) {
+        CSSValueImpl *value = getPropertyCSSValue(set[i]);
+        if (value)
+            m_values.remove(CSSProperty(set[i], value, false));
+    }
+}
+
 CSSMutableStyleDeclarationImpl *CSSMutableStyleDeclarationImpl::makeMutable()
 {
     return this;
@@ -1192,4 +1212,9 @@ DOMString CSSProperty::cssText() const
     return getPropertyName(m_id) + DOMString(": ") + m_value->cssText() + (m_bImportant ? DOMString(" !important") : DOMString()) + DOMString("; ");
 }
 
+bool operator==(const CSSProperty &a, const CSSProperty &b)
+{
+    return a.m_id == b.m_id && a.m_bImportant == b.m_bImportant && a.m_value == b.m_value;
+}
+
 }
index eab8849a00dad09c810e47d46fb0e791f1e6941f..389a2d80a1adf9cf8776ff2c007b039fe0d8e886 100644 (file)
@@ -422,6 +422,8 @@ public:
     int  m_id;
     bool m_bImportant;
 
+    friend bool operator==(const CSSProperty &, const CSSProperty &);
+
 protected:
     CSSValueImpl *m_value;
 };
@@ -466,6 +468,8 @@ public:
     DOMString removeProperty(int propertyID, bool notifyChanged = true)
         { int exceptionCode; return removeProperty(propertyID, notifyChanged, exceptionCode); }
 
+    void clear();
+
     void setChanged();
  
     // setLengthProperty treats integers as pixels! (Needed for conversion of HTML attributes.)
@@ -481,6 +485,8 @@ public:
     void addParsedProperties(const CSSProperty * const *, int numProperties);
  
     CSSMutableStyleDeclarationImpl *copyBlockProperties() const;
+    void removeBlockProperties();
+    void removePropertiesInSet(const int *set, unsigned length);
 
     void merge(CSSMutableStyleDeclarationImpl *, bool argOverridesOnConflict = true);
  
index 6254b45055e7d7658e0784b8bde6f17fefb26cde..4672abeb19ff252f22185f1821e47fd3662c5227 100644 (file)
@@ -178,18 +178,12 @@ static DOMString &styleSpanClassString()
 
 static bool isEmptyStyleSpan(const NodeImpl *node)
 {
-    if (!node || !node->isHTMLElement())
+    if (!node || !node->isHTMLElement() || node->id() != ID_SPAN)
         return false;
 
     const HTMLElementImpl *elem = static_cast<const HTMLElementImpl *>(node);
     CSSMutableStyleDeclarationImpl *inlineStyleDecl = elem->inlineStyleDecl();
-    if (!inlineStyleDecl || inlineStyleDecl->length() == 0) {
-        NamedAttrMapImpl *map = elem->attributes();
-        if (map && map->length() == 1 && elem->getAttribute(ATTR_CLASS) == styleSpanClassString())
-            return true;
-    }
-    
-    return false;
+    return (!inlineStyleDecl || inlineStyleDecl->length() == 0) && elem->getAttribute(ATTR_CLASS) == styleSpanClassString();
 }
 
 static bool isStyleSpan(const NodeImpl *node)
@@ -198,7 +192,7 @@ static bool isStyleSpan(const NodeImpl *node)
         return false;
 
     const HTMLElementImpl *elem = static_cast<const HTMLElementImpl *>(node);
-    return elem->getAttribute(ATTR_CLASS) == styleSpanClassString();
+    return elem->id() == ID_SPAN && elem->getAttribute(ATTR_CLASS) == styleSpanClassString();
 }
 
 static DOMString &blockPlaceholderClassString()
@@ -1545,17 +1539,8 @@ void ApplyStyleCommand::removeCSSStyle(CSSMutableStyleDeclarationImpl *style, HT
         }
     }
 
-    if (elem->id() == ID_SPAN && elem->renderer() && elem->renderer()->isInline()) {
-        // Check to see if the span is one we added to apply style.
-        // If it is, and there are no more attributes on the span other than our
-        // class marker, remove the span.
-        if (decl->length() == 0) {
-            removeNodeAttribute(elem, ATTR_STYLE);
-            NamedAttrMapImpl *map = elem->attributes();
-            if (map && map->length() == 1 && elem->getAttribute(ATTR_CLASS) == styleSpanClassString())
-                removeNodePreservingChildren(elem);
-        }
-    }
+    if (isEmptyStyleSpan(elem))
+        removeNodePreservingChildren(elem);
 }
 
 void ApplyStyleCommand::removeBlockStyle(CSSMutableStyleDeclarationImpl *style, const Position &start, const Position &end)
@@ -3933,14 +3918,18 @@ void RemoveNodePreservingChildrenCommand::doApply()
 //------------------------------------------------------------------------------------------
 // ReplaceSelectionCommand
 
-ReplacementFragment::ReplacementFragment(DocumentFragmentImpl *fragment)
-    : m_fragment(fragment), m_hasInterchangeNewline(false), m_hasMoreThanOneBlock(false)
+ReplacementFragment::ReplacementFragment(DocumentImpl *document, DocumentFragmentImpl *fragment)
+    : m_document(document), m_fragment(fragment), m_hasInterchangeNewline(false), m_hasMoreThanOneBlock(false)
 {
+    if (!m_document)
+        return;
+
     if (!m_fragment) {
         m_type = EmptyFragment;
         return;
     }
 
+    m_document->ref();
     m_fragment->ref();
 
     NodeImpl *firstChild = m_fragment->firstChild();
@@ -3958,30 +3947,6 @@ ReplacementFragment::ReplacementFragment(DocumentFragmentImpl *fragment)
     
     m_type = TreeFragment;
 
-    // look for default style
-    if (firstChild == lastChild && isStyleSpan(firstChild)) {
-        NodeImpl *styleSpan = firstChild;
-        DOMString styleString = static_cast<ElementImpl *>(styleSpan)->getAttribute(ATTR_STYLE);
-        if (styleString.length() > 0) {
-            CSSParser parser(true);
-            m_defaultStyle = new CSSMutableStyleDeclarationImpl;
-            if (!parser.parseDeclaration(m_defaultStyle, styleString)) {
-                m_defaultStyle->deref();
-                m_defaultStyle = 0;
-            }
-        }
-        NodeImpl *n = styleSpan->firstChild();
-        while (n) {
-            NodeImpl *next = n->traverseNextSibling();
-            n->ref();
-            removeNode(n);
-            insertNodeBefore(n, styleSpan);
-            n->deref();
-            n = next;
-        }
-        removeNode(styleSpan);
-    }
-
     NodeImpl *node = m_fragment->firstChild();
     int realBlockCount = 0;
     NodeImpl *nodeToDelete = 0;
@@ -4021,10 +3986,21 @@ ReplacementFragment::ReplacementFragment(DocumentFragmentImpl *fragment)
 
      if (blockCount > 1)
         m_hasMoreThanOneBlock = true;
+        
+    // Prepare this fragment to merge styles correctly into the destination.
+    computeStylesForNodes();
+    removeStyleNodes();
 }
 
 ReplacementFragment::~ReplacementFragment()
 {
+    QMapIterator<NodeImpl *, CSSMutableStyleDeclarationImpl *> it;
+    for (it = m_styles.begin(); it != m_styles.end(); ++it) {
+        it.key()->deref();
+        it.data()->deref();
+    }
+    if (m_document)
+        m_document->deref();
     if (m_fragment)
         m_fragment->deref();
 }
@@ -4039,6 +4015,12 @@ NodeImpl *ReplacementFragment::lastChild() const
     return  m_fragment->lastChild(); 
 }
 
+CSSMutableStyleDeclarationImpl *ReplacementFragment::styleForNode(NodeImpl *node)
+{
+    QMapConstIterator<NodeImpl *,CSSMutableStyleDeclarationImpl *> it = m_styles.find(node);
+    return it != m_styles.end() ? *it : 0;
+}
+
 NodeImpl *ReplacementFragment::mergeStartNode() const
 {
     NodeImpl *node = m_fragment->firstChild();
@@ -4089,6 +4071,20 @@ NodeImpl *ReplacementFragment::enclosingBlock(NodeImpl *node) const
     return node ? node : m_fragment;
 }
 
+void ReplacementFragment::removeNodePreservingChildren(NodeImpl *node)
+{
+    if (!node)
+        return;
+
+    while (NodeImpl *n = node->firstChild()) {
+        n->ref();
+        removeNode(n);
+        insertNodeBefore(n, node);
+        n->deref();
+    }
+    removeNode(node);
+}
+
 void ReplacementFragment::removeNode(NodeImpl *node)
 {
     if (!node)
@@ -4115,8 +4111,79 @@ void ReplacementFragment::insertNodeBefore(NodeImpl *node, NodeImpl *refNode)
     int exceptionCode = 0;
     parent->insertBefore(node, refNode, exceptionCode);
     ASSERT(exceptionCode == 0);
- }
+}
+
+void ReplacementFragment::computeStylesForNodes()
+{
+    // This function inserts the nodes from the fragment into the destination
+    // document and computes the style for each. This information will be
+    // used later to apply the necessary styles to the nodes after placing
+    // them in their intended location.
+    NodeImpl *body = m_document->body();
+    if (!body)
+        return;
+
+    ElementImpl *holder = createDefaultParagraphElement(m_document);
+    holder->ref();
+
+    int exceptionCode = 0;
+    holder->appendChild(m_fragment, exceptionCode);
+    ASSERT(exceptionCode == 0);
+    body->appendChild(holder, exceptionCode);
+    ASSERT(exceptionCode == 0);
+    m_document->updateLayout();
+    for (NodeImpl *node = holder->firstChild(); node; node = node->traverseNextNode()) {
+        CSSComputedStyleDeclarationImpl *computedStyle = Position(node, 0).computedStyle();
+        computedStyle->ref();
+        CSSMutableStyleDeclarationImpl *style = computedStyle->copyInheritableProperties();
+        style->ref();
+        node->ref();
+        m_styles[node] = style;
+        computedStyle->deref();
+    }
+    body->removeChild(holder, exceptionCode);
+    ASSERT(exceptionCode == 0);
+    m_document->updateLayout();
+
+    while (NodeImpl *node = holder->firstChild()) {
+        node->ref();
+        holder->removeChild(node, exceptionCode);
+        ASSERT(exceptionCode == 0);
+        m_fragment->appendChild(node, exceptionCode);
+        ASSERT(exceptionCode == 0);
+        node->deref();
+    }
 
+    holder->deref();
+}
+
+void ReplacementFragment::removeStyleNodes()
+{
+    // Since style information has been computed and cached away in
+    // computeStylesForNodes(), these style nodes can be removed, since
+    // the correct styles will be added back in applyStyleToInsertedNodes().
+    NodeImpl *node = m_fragment->firstChild();
+    while (node) {
+        NodeImpl *next = node->traverseNextNode();
+        // This list of tags change the appearance of content
+        // in ways we can add back on later with CSS, if necessary.
+        if (node->id() == ID_BIG || 
+            node->id() == ID_CENTER || 
+            node->id() == ID_FONT || 
+            node->id() == ID_I || 
+            node->id() == ID_S || 
+            node->id() == ID_SMALL || 
+            node->id() == ID_STRIKE || 
+            node->id() == ID_SUB || 
+            node->id() == ID_SUP || 
+            node->id() == ID_TT || 
+            node->id() == ID_U || 
+            isStyleSpan(node)) {
+            removeNodePreservingChildren(node);
+        }
+        node = next;
+    }
+}
 
 bool isProbablyBlock(const NodeImpl *node)
 {
@@ -4151,7 +4218,7 @@ bool isProbablyBlock(const NodeImpl *node)
 
 ReplaceSelectionCommand::ReplaceSelectionCommand(DocumentImpl *document, DocumentFragmentImpl *fragment, bool selectReplacement, bool smartReplace) 
     : CompositeEditCommand(document), 
-      m_fragment(fragment),
+      m_fragment(document, fragment),
       m_firstNodeInserted(0),
       m_lastNodeInserted(0),
       m_lastTopNodeInserted(0),
@@ -4406,6 +4473,8 @@ void ReplaceSelectionCommand::doApply()
             }
         }
     
+        applyStyleToInsertedNodes();
+    
         completeHTMLReplacement();
     }
     
@@ -4517,6 +4586,55 @@ void ReplaceSelectionCommand::updateNodesInserted(NodeImpl *node)
         old->deref();
 }
 
+void ReplaceSelectionCommand::applyStyleToInsertedNodes()
+{
+    // This function uses the cached style information computed in by the 
+    // ReplacementFragment class to apply the styles necessary to make
+    // the document look right.
+
+    document()->updateLayout();
+
+    NodeImpl *node = m_firstNodeInserted;
+    NodeImpl *beyondEnd = m_lastNodeInserted->traverseNextNode();
+    while (node && node != beyondEnd) {
+        NodeImpl *next = node->traverseNextNode();
+        
+        // See if this node was present in the fragment, or was added by the paste algorithm. 
+        // If it was in the fragment, then we need to check, and perhaps fix up, its style.
+        CSSMutableStyleDeclarationImpl *desiredStyle = m_fragment.styleForNode(node);
+        if (desiredStyle) {
+            // The desiredStyle declaration tells what style this node wants to be.
+            // Compare that to the style that it is right now in the document.
+            Position pos(node, 0);
+            CSSComputedStyleDeclarationImpl *currentStyle = pos.computedStyle();
+            currentStyle->ref();
+            currentStyle->diff(desiredStyle);
+            
+            // Only add in block perperties if the node is at the start of a 
+            // paragraph. This matches AppKit.
+            if (!isStartOfParagraph(VisiblePosition(pos, DOWNSTREAM)))
+                desiredStyle->removeBlockProperties();
+            
+            // If the desiredStyle is non-zero length, that means the current style differs
+            // from the desired by the styles remaining in the desiredStyle declaration.
+            if (desiredStyle->length() > 0) {
+                DOM::RangeImpl *rangeAroundNode = document()->createRange();
+                rangeAroundNode->ref();
+                int exceptionCode = 0;
+                rangeAroundNode->selectNode(node, exceptionCode);
+                ASSERT(exceptionCode == 0);
+                setEndingSelection(Selection(rangeAroundNode, DOWNSTREAM, UPSTREAM));
+                applyStyle(desiredStyle);
+                rangeAroundNode->deref();
+            }
+
+            currentStyle->deref();
+        }
+
+        node = next;
+    }
+}
+
 //------------------------------------------------------------------------------------------
 // SetNodeAttributeCommand
 
index aa8bca1c15db775fb91c5a6316e48b24bf254369..ea21ab175cb0dce0527ba5e19040ab6a61bc0f75 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "dom_nodeimpl.h"
 #include "editing/edit_actions.h"
+#include "qmap.h"
 #include "qptrlist.h"
 #include "qvaluelist.h"
 #include "selection.h"
@@ -659,7 +660,7 @@ private:
 class ReplacementFragment
 {
 public:
-    ReplacementFragment(DOM::DocumentFragmentImpl *fragment);
+    ReplacementFragment(DOM::DocumentImpl *, DOM::DocumentFragmentImpl *);
     ~ReplacementFragment();
 
     enum EFragmentType { EmptyFragment, SingleTextNodeFragment, TreeFragment };
@@ -669,7 +670,9 @@ public:
     DOM::NodeImpl *lastChild() const;
 
     DOM::NodeImpl *mergeStartNode() const;
-    
+
+    DOM::CSSMutableStyleDeclarationImpl *styleForNode(DOM::NodeImpl *node);
+        
     void pruneEmptyNodes();
 
     EFragmentType type() const { return m_type; }
@@ -688,14 +691,19 @@ private:
     static bool isInterchangeNewlineNode(const DOM::NodeImpl *);
     static bool isInterchangeConvertedSpaceSpan(const DOM::NodeImpl *);
 
+    void computeStylesForNodes();
+    void removeStyleNodes();
+
     // A couple simple DOM helpers
     DOM::NodeImpl *enclosingBlock(DOM::NodeImpl *) const;
     void removeNode(DOM::NodeImpl *);
+    void removeNodePreservingChildren(DOM::NodeImpl *);
     void insertNodeBefore(DOM::NodeImpl *node, DOM::NodeImpl *refNode);
 
     EFragmentType m_type;
-    DOM::CSSMutableStyleDeclarationImpl *m_defaultStyle;
+    DOM::DocumentImpl *m_document;
     DOM::DocumentFragmentImpl *m_fragment;
+    QMap<DOM::NodeImpl *, DOM::CSSMutableStyleDeclarationImpl *> m_styles;
     bool m_hasInterchangeNewline;
     bool m_hasMoreThanOneBlock;
 };
@@ -721,7 +729,8 @@ private:
     void insertNodeBeforeAndUpdateNodesInserted(DOM::NodeImpl *insertChild, DOM::NodeImpl *refChild);
 
     void updateNodesInserted(DOM::NodeImpl *);
-    
+    void applyStyleToInsertedNodes();
+
     ReplacementFragment m_fragment;
     DOM::NodeImpl *m_firstNodeInserted;
     DOM::NodeImpl *m_lastNodeInserted;
index 5c7eb1ef85bbaf462927673a232c82751a890fd8..407771e2637eae706af7eb09ad7cc77d0179548a 100644 (file)
@@ -157,7 +157,7 @@ static QString renderedText(const NodeImpl *node, const RangeImpl *range)
     return result;
 }
 
-static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnotateForInterchange annotate)
+static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnotateForInterchange annotate, CSSMutableStyleDeclarationImpl *defaultStyle)
 {
     unsigned short type = node->nodeType();
     switch (type) {
@@ -170,9 +170,21 @@ static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnota
                         return stringValueForRange(node, range);
                 }
             }
-            if (annotate)
-                return convertHTMLTextToInterchangeFormat(escapeHTML(renderedText(node, range))); 
-            return escapeHTML(stringValueForRange(node, range));
+            QString markup = annotate ? escapeHTML(renderedText(node, range)) : escapeHTML(stringValueForRange(node, range));            
+            if (defaultStyle) {
+                NodeImpl *element = node->parentNode();
+                if (element) {
+                    CSSMutableStyleDeclarationImpl *style = Position(element, 0).computedStyle()->copyInheritableProperties();
+                    style->ref();
+                    defaultStyle->diff(style);
+                    if (style->length() > 0) {
+                        QString openTag = QString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + style->cssText().string() + "\">";
+                        markup = openTag + markup + "</span>";
+                    }
+                    style->deref();
+                }            
+            }
+            return annotate ? convertHTMLTextToInterchangeFormat(markup) : markup;
         }
         case Node::COMMENT_NODE:
             return static_cast<const CommentImpl *>(node)->toString().string();
@@ -182,11 +194,32 @@ static QString startMarkup(const NodeImpl *node, const RangeImpl *range, EAnnota
             QString markup = QChar('<') + node->nodeName().string();
             if (type == Node::ELEMENT_NODE) {
                 const ElementImpl *el = static_cast<const ElementImpl *>(node);
+                DOMString additionalStyle;
+                if (defaultStyle && el->isHTMLElement()) {
+                    CSSMutableStyleDeclarationImpl *style = Position(const_cast<ElementImpl *>(el), 0).computedStyle()->copyInheritableProperties();
+                    style->ref();
+                    defaultStyle->diff(style);
+                    if (style->length() > 0) {
+                        CSSMutableStyleDeclarationImpl *inlineStyleDecl = static_cast<const HTMLElementImpl *>(el)->inlineStyleDecl();
+                        if (inlineStyleDecl)
+                            inlineStyleDecl->diff(style);
+                        additionalStyle = style->cssText();
+                    }
+                    style->deref();
+                }
                 NamedAttrMapImpl *attrs = el->attributes();
                 unsigned long length = attrs->length();
-                for (unsigned int i=0; i<length; i++) {
-                    AttributeImpl *attr = attrs->attributeItem(i);
-                    markup += " " + node->getDocument()->attrName(attr->id()).string() + "=\"" + attr->value().string() + "\"";
+                if (length == 0 && additionalStyle.length() > 0) {
+                    markup += " " + node->getDocument()->attrName(ATTR_STYLE).string() + "=\"" + additionalStyle.string() + "\"";
+                }
+                else {
+                    for (unsigned int i=0; i<length; i++) {
+                        AttributeImpl *attr = attrs->attributeItem(i);
+                        DOMString value = attr->value();
+                        if (attr->id() == ATTR_STYLE && additionalStyle.length() > 0)
+                            value += "; " + additionalStyle;
+                        markup += " " + node->getDocument()->attrName(attr->id()).string() + "=\"" + value.string() + "\"";
+                    }
                 }
             }
             markup += node->isHTMLElement() ? ">" : "/>";
@@ -213,7 +246,7 @@ static QString markup(const NodeImpl *startNode, bool onlyIncludeChildren, bool
             if (nodes) {
                 nodes->append(current);
             }
-            me += startMarkup(current, 0, DoNotAnnotateForInterchange);
+            me += startMarkup(current, 0, DoNotAnnotateForInterchange, 0);
         }        
         if (!current->isHTMLElement() || endTag[current->id()] != FORBIDDEN) {
             // print children
@@ -266,6 +299,11 @@ QString createMarkup(const RangeImpl *range, QPtrList<NodeImpl> *nodes, EAnnotat
     NodeImpl *pastEnd = range->pastEndNode();
     NodeImpl *lastClosed = 0;
     QPtrList<NodeImpl> ancestorsToClose;
+
+    // calculate the "default style" for this markup
+    Position pos(commonAncestor->getDocument()->documentElement(), 0);
+    CSSMutableStyleDeclarationImpl *defaultStyle = pos.computedStyle()->copyInheritableProperties();
+    defaultStyle->ref();
     
     // Iterate through the nodes of the range.
     NodeImpl *next;
@@ -280,7 +318,7 @@ QString createMarkup(const RangeImpl *range, QPtrList<NodeImpl> *nodes, EAnnotat
         }
         
         // Add the node to the markup.
-        markups.append(startMarkup(n, range, annotate));
+        markups.append(startMarkup(n, range, annotate, defaultStyle));
         if (nodes) {
             nodes->append(n);
         }
@@ -309,7 +347,7 @@ QString createMarkup(const RangeImpl *range, QPtrList<NodeImpl> *nodes, EAnnotat
                         NodeImpl *nextParent = next->parentNode();
                         if (n != nextParent) {
                             for (NodeImpl *parent = n->parent(); parent != 0 && parent != nextParent; parent = parent->parentNode()) {
-                                markups.prepend(startMarkup(parent, range, annotate));
+                                markups.prepend(startMarkup(parent, range, annotate, defaultStyle));
                                 markups.append(endMarkup(parent));
                                 if (nodes) {
                                     nodes->append(parent);
@@ -338,7 +376,7 @@ QString createMarkup(const RangeImpl *range, QPtrList<NodeImpl> *nodes, EAnnotat
                 break;
             }
         }
-        markups.prepend(startMarkup(ancestor, range, annotate));
+        markups.prepend(startMarkup(ancestor, range, annotate, defaultStyle));
         markups.append(endMarkup(ancestor));
         if (nodes) {
             nodes->append(ancestor);
@@ -359,13 +397,10 @@ QString createMarkup(const RangeImpl *range, QPtrList<NodeImpl> *nodes, EAnnotat
     }
 
     // add in the "default style" for this markup
-    Position pos(commonAncestor->getDocument()->documentElement(), 0);
-    CSSMutableStyleDeclarationImpl *style = pos.computedStyle()->copyInheritableProperties();
-    style->ref();
-    QString openTag = QString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + style->cssText().string() + "\">";
+    QString openTag = QString("<span class=\"") + AppleStyleSpanClass + "\" style=\"" + defaultStyle->cssText().string() + "\">";
     markups.prepend(openTag);
     markups.append("</span>");
-    style->deref();
+    defaultStyle->deref();
 
     return markups.join("");
 }