Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Nov 2004 17:56:33 +0000 (17:56 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Nov 2004 17:56:33 +0000 (17:56 +0000)
        Fix for this bug:

        <rdar://problem/3775920> REGRESSION (Mail): Centering doesn't work in HTML mail

        * khtml/css/css_computedstyle.cpp:
        (DOM::CSSComputedStyleDeclarationImpl::copyInheritableProperties): Factor out the
        implementation here into new copyPropertiesInSet helper. This now calls the
        generalized copyPropertiesInSet function with the arguments needed to make copying
        inheritable work.
        * khtml/css/css_computedstyle.h:
        * khtml/css/css_valueimpl.cpp:
        (CSSStyleDeclarationImpl::diff): Move this function here from css_computedstyle.cpp.
        In order to do apply block properties, "regular" style declarations need to do style
        diffs as well.
        (CSSStyleDeclarationImpl::copyBlockProperties): New helper. Just like copyInheritableProperties
        except that it uses a different set of properties that apply only to blocks.
        (CSSStyleDeclarationImpl::copyPropertiesInSet): New helper that looks at a style declaration
        and copies out those properties listed in a pre-defined set.
        * khtml/css/css_valueimpl.h:
        * khtml/editing/htmlediting.cpp:
        (khtml::StyleChange::StyleChange): Modified to work with style changes that apply to a whole
        block, factoring out some of the special case code that should now only run in the inline case.
        (khtml::StyleChange::init): Factored out the code that now is in checkForLegacyHTMLStyleChange.
        (khtml::StyleChange::checkForLegacyHTMLStyleChange): New helper for case where we want
        special handling for "legacy" HTML styles like <B> and <I>.
        (khtml::ApplyStyleCommand::doApply): Much refactoring in this class to divide up the work of
        style changes into different kinds. CSS specifies certain properties only apply to certain
        element types. This set of changes now recognizes two such separate cases: styles that apply
        to blocks, and styles that apply to inlines.
        (khtml::ApplyStyleCommand::applyBlockStyle): New function to handle apply styles to whole blocks.
        (khtml::ApplyStyleCommand::applyInlineStyle): New function to handle apply styles to inlines.
        (khtml::ApplyStyleCommand::isHTMLStyleNode): Is now passed a CSSStyleDeclarationImpl to work
        with rather than working on the CSSStyleDeclarationImpl member variable of the class. This is
        done so that the function can be passed a portion of the styles being applied so that block styles
        and inline styles can be handled separately.
        (khtml::ApplyStyleCommand::removeCSSStyle): Ditto.
        (khtml::ApplyStyleCommand::removeBlockStyle): New function to handle removing styles from whole blocks.
        (khtml::ApplyStyleCommand::removeInlineStyle): New function to removing styles from inlines.
        (khtml::ApplyStyleCommand::addBlockStyleIfNeeded): New function to handle applying style to whole blocks.
        (khtml::ApplyStyleCommand::addInlineStyleIfNeeded): New function to handle applying style to inlines.
        * khtml/editing/htmlediting.h:
        (khtml::StyleChange::): Changed as described above.
        (khtml::StyleChange::usesLegacyStyles):
        (khtml::EditCommand::setEndingSelectionNeedsLayout): New function to that tells the ending selection
        it needs to layout, even though it has not changed position in the DOM. For instance, this is needed
        when text align changes.
        * khtml/khtml_part.cpp:
        (KHTMLPart::setTypingStyle): Put in an early bail-out in the case where the current style matches
        the passed-in argument.
        (KHTMLPart::applyStyle): Modify this function so that block styles are applied when the selection
        is a caret. Formerly, this just set typing style and made no visible changes to the document.

        New tests.

        * layout-tests/editing/editing.js: Added some glue to change text align.
        * layout-tests/editing/style/block-style-001-expected.txt: Added.
        * layout-tests/editing/style/block-style-001.html: Added.
        * layout-tests/editing/style/block-style-002-expected.txt: Added.
        * layout-tests/editing/style/block-style-002.html: Added.
        * layout-tests/editing/style/block-style-003-expected.txt: Added.
        * layout-tests/editing/style/block-style-003.html: Added.

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

15 files changed:
LayoutTests/editing/editing.js
LayoutTests/editing/style/block-style-001-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/block-style-001.html [new file with mode: 0644]
LayoutTests/editing/style/block-style-002-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/block-style-002.html [new file with mode: 0644]
LayoutTests/editing/style/block-style-003-expected.txt [new file with mode: 0644]
LayoutTests/editing/style/block-style-003.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/css/css_computedstyle.cpp
WebCore/khtml/css/css_computedstyle.h
WebCore/khtml/css/css_valueimpl.cpp
WebCore/khtml/css/css_valueimpl.h
WebCore/khtml/editing/htmlediting.cpp
WebCore/khtml/editing/htmlediting.h
WebCore/khtml/khtml_part.cpp

index fab1f41..ee5c08e 100644 (file)
@@ -221,6 +221,54 @@ function italicCommand() {
 
 //-------------------------------------------------------------------------------------------------------
 
+function execJustifyCenterCommand() {
+    document.execCommand("JustifyCenter");
+}
+function justifyCenterCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execJustifyCenterCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execJustifyCenterCommand();
+    }
+}
+
+
+//-------------------------------------------------------------------------------------------------------
+
+function execJustifyLeftCommand() {
+    document.execCommand("JustifyLeft");
+}
+function justifyLeftCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execJustifyLeftCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execJustifyLeftCommand();
+    }
+}
+
+
+//-------------------------------------------------------------------------------------------------------
+
+function execJustifyRightCommand() {
+    document.execCommand("JustifyRight");
+}
+function justifyRightCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execJustifyRightCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execJustifyRightCommand();
+    }
+}
+
+
+//-------------------------------------------------------------------------------------------------------
+
 function execInsertNewlineCommand() {
     document.execCommand("InsertNewline");
 }
diff --git a/LayoutTests/editing/style/block-style-001-expected.txt b/LayoutTests/editing/style/block-style-001-expected.txt
new file mode 100644 (file)
index 0000000..7342e81
--- /dev/null
@@ -0,0 +1,25 @@
+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 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (202,14) size 284x28
+          text run at (202,14) width 284: "Here is some text. Should be "
+        RenderInline {B} at (0,0) size 89x28
+          RenderText {TEXT} at (486,14) size 89x28
+            text run at (486,14) width 89: "centered"
+        RenderText {TEXT} at (575,14) size 6x28
+          text run at (575,14) width 6: "."
+      RenderBlock {DIV} at (0,56) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 284x28
+          text run at (14,14) width 284: "Here is some text. Should be "
+        RenderInline {B} at (0,0) size 90x28
+          RenderText {TEXT} at (298,14) size 90x28
+            text run at (298,14) width 90: "flush left"
+        RenderText {TEXT} at (388,14) size 6x28
+          text run at (388,14) width 6: "."
+selection is CARET:
+start:      position 1 of child 1 {TEXT} of child 1 {DIV} of root {BODY}
+upstream:   position 0 of child 1 {DIV} of root {BODY}
+downstream: position 1 of child 1 {TEXT} of child 1 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/style/block-style-001.html b/LayoutTests/editing/style/block-style-001.html
new file mode 100644 (file)
index 0000000..6b3e0dc
--- /dev/null
@@ -0,0 +1,35 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    justifyCenterCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body id="root" contenteditable>
+<div id="test" class="editing">
+Here is some text. Should be <b>centered</b>.
+</div>
+<div id="test" class="editing">
+Here is some text. Should be <b>flush left</b>.
+</div>
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/style/block-style-002-expected.txt b/LayoutTests/editing/style/block-style-002-expected.txt
new file mode 100644 (file)
index 0000000..7342e81
--- /dev/null
@@ -0,0 +1,25 @@
+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 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (202,14) size 284x28
+          text run at (202,14) width 284: "Here is some text. Should be "
+        RenderInline {B} at (0,0) size 89x28
+          RenderText {TEXT} at (486,14) size 89x28
+            text run at (486,14) width 89: "centered"
+        RenderText {TEXT} at (575,14) size 6x28
+          text run at (575,14) width 6: "."
+      RenderBlock {DIV} at (0,56) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (14,14) size 284x28
+          text run at (14,14) width 284: "Here is some text. Should be "
+        RenderInline {B} at (0,0) size 90x28
+          RenderText {TEXT} at (298,14) size 90x28
+            text run at (298,14) width 90: "flush left"
+        RenderText {TEXT} at (388,14) size 6x28
+          text run at (388,14) width 6: "."
+selection is CARET:
+start:      position 1 of child 1 {TEXT} of child 1 {DIV} of root {BODY}
+upstream:   position 0 of child 1 {DIV} of root {BODY}
+downstream: position 1 of child 1 {TEXT} of child 1 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/style/block-style-002.html b/LayoutTests/editing/style/block-style-002.html
new file mode 100644 (file)
index 0000000..90d3eda
--- /dev/null
@@ -0,0 +1,35 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    justifyCenterCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body id="root" contenteditable>
+<div style="test-align: right;" id="test" class="editing">
+Here is some text. Should be <b>centered</b>.
+</div>
+<div id="test" class="editing">
+Here is some text. Should be <b>flush left</b>.
+</div>
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
diff --git a/LayoutTests/editing/style/block-style-003-expected.txt b/LayoutTests/editing/style/block-style-003-expected.txt
new file mode 100644 (file)
index 0000000..0ee67b3
--- /dev/null
@@ -0,0 +1,28 @@
+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 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (202,14) size 284x28
+          text run at (202,14) width 284: "Here is some text. Should be "
+        RenderInline {B} at (0,0) size 89x28
+          RenderText {TEXT} at (486,14) size 89x28
+            text run at (486,14) width 89: "centered"
+        RenderText {TEXT} at (575,14) size 6x28
+          text run at (575,14) width 6: "."
+      RenderBlock {DIV} at (0,56) size 784x56 [border: (2px solid #FF0000)]
+        RenderText {TEXT} at (202,14) size 284x28
+          text run at (202,14) width 284: "Here is some text. Should be "
+        RenderInline {B} at (0,0) size 89x28
+          RenderText {TEXT} at (486,14) size 89x28
+            text run at (486,14) width 89: "centered"
+        RenderText {TEXT} at (575,14) size 6x28
+          text run at (575,14) width 6: "."
+selection is RANGE:
+start:      position 1 of child 1 {TEXT} of child 1 {DIV} of root {BODY}
+upstream:   position 0 of child 1 {DIV} of root {BODY}
+downstream: position 1 of child 1 {TEXT} of child 1 {DIV} of root {BODY}
+end:        position 12 of child 1 {TEXT} of child 3 {DIV} of root {BODY}
+upstream:   position 12 of child 1 {TEXT} of child 3 {DIV} of root {BODY}
+downstream: position 12 of child 1 {TEXT} of child 3 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/style/block-style-003.html b/LayoutTests/editing/style/block-style-003.html
new file mode 100644 (file)
index 0000000..7818a4c
--- /dev/null
@@ -0,0 +1,37 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 51; i++)
+        extendSelectionForwardByCharacterCommand();
+    justifyCenterCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body id="root" contenteditable>
+<div style="test-align: right;" id="test" class="editing">
+Here is some text. Should be <b>centered</b>.
+</div>
+<div id="test" class="editing">
+Here is some text. Should be <b>centered</b>.
+</div>
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 0d0462b..abe2fd5 100644 (file)
@@ -1,3 +1,69 @@
+2004-11-01  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/3775920> REGRESSION (Mail): Centering doesn't work in HTML mail
+
+        * khtml/css/css_computedstyle.cpp:
+        (DOM::CSSComputedStyleDeclarationImpl::copyInheritableProperties): Factor out the 
+        implementation here into new copyPropertiesInSet helper. This now calls the
+        generalized copyPropertiesInSet function with the arguments needed to make copying
+        inheritable work.
+        * khtml/css/css_computedstyle.h:
+        * khtml/css/css_valueimpl.cpp:
+        (CSSStyleDeclarationImpl::diff): Move this function here from css_computedstyle.cpp.
+        In order to do apply block properties, "regular" style declarations need to do style
+        diffs as well.
+        (CSSStyleDeclarationImpl::copyBlockProperties): New helper. Just like copyInheritableProperties
+        except that it uses a different set of properties that apply only to blocks.
+        (CSSStyleDeclarationImpl::copyPropertiesInSet): New helper that looks at a style declaration
+        and copies out those properties listed in a pre-defined set.
+        * khtml/css/css_valueimpl.h:
+        * khtml/editing/htmlediting.cpp:
+        (khtml::StyleChange::StyleChange): Modified to work with style changes that apply to a whole
+        block, factoring out some of the special case code that should now only run in the inline case.
+        (khtml::StyleChange::init): Factored out the code that now is in checkForLegacyHTMLStyleChange.
+        (khtml::StyleChange::checkForLegacyHTMLStyleChange): New helper for case where we want
+        special handling for "legacy" HTML styles like <B> and <I>.
+        (khtml::ApplyStyleCommand::doApply): Much refactoring in this class to divide up the work of
+        style changes into different kinds. CSS specifies certain properties only apply to certain
+        element types. This set of changes now recognizes two such separate cases: styles that apply
+        to blocks, and styles that apply to inlines.
+        (khtml::ApplyStyleCommand::applyBlockStyle): New function to handle apply styles to whole blocks.
+        (khtml::ApplyStyleCommand::applyInlineStyle): New function to handle apply styles to inlines.
+        (khtml::ApplyStyleCommand::isHTMLStyleNode): Is now passed a CSSStyleDeclarationImpl to work
+        with rather than working on the CSSStyleDeclarationImpl member variable of the class. This is
+        done so that the function can be passed a portion of the styles being applied so that block styles
+        and inline styles can be handled separately.
+        (khtml::ApplyStyleCommand::removeCSSStyle): Ditto.
+        (khtml::ApplyStyleCommand::removeBlockStyle): New function to handle removing styles from whole blocks.
+        (khtml::ApplyStyleCommand::removeInlineStyle): New function to removing styles from inlines.
+        (khtml::ApplyStyleCommand::addBlockStyleIfNeeded): New function to handle applying style to whole blocks.
+        (khtml::ApplyStyleCommand::addInlineStyleIfNeeded): New function to handle applying style to inlines.
+        * khtml/editing/htmlediting.h:
+        (khtml::StyleChange::): Changed as described above.
+        (khtml::StyleChange::usesLegacyStyles):
+        (khtml::EditCommand::setEndingSelectionNeedsLayout): New function to that tells the ending selection
+        it needs to layout, even though it has not changed position in the DOM. For instance, this is needed 
+        when text align changes.
+        * khtml/khtml_part.cpp:
+        (KHTMLPart::setTypingStyle): Put in an early bail-out in the case where the current style matches
+        the passed-in argument.
+        (KHTMLPart::applyStyle): Modify this function so that block styles are applied when the selection
+        is a caret. Formerly, this just set typing style and made no visible changes to the document.
+
+        New tests.
+
+        * layout-tests/editing/editing.js: Added some glue to change text align.
+        * layout-tests/editing/style/block-style-001-expected.txt: Added.
+        * layout-tests/editing/style/block-style-001.html: Added.
+        * layout-tests/editing/style/block-style-002-expected.txt: Added.
+        * layout-tests/editing/style/block-style-002.html: Added.
+        * layout-tests/editing/style/block-style-003-expected.txt: Added.
+        * layout-tests/editing/style/block-style-003.html: Added.
+
 === Safari-169 ===
 
 2004-10-29  Darin Adler  <darin@apple.com>
index 29ed3fb..9342d66 100644 (file)
@@ -917,36 +917,7 @@ CSSProperty CSSComputedStyleDeclarationImpl::property(int id) const
 
 CSSStyleDeclarationImpl *CSSComputedStyleDeclarationImpl::copyInheritableProperties() const
 {
-    QPtrList<CSSProperty> *list = new QPtrList<CSSProperty>;
-    list->setAutoDelete(true);
-    for (unsigned i = 0; i < sizeof(InheritableProperties) / sizeof(InheritableProperties[0]); i++) {
-        CSSValueImpl *value = getPropertyCSSValue(InheritableProperties[i]);
-        if (value) {
-            CSSProperty *property = new CSSProperty;
-            property->m_id = InheritableProperties[i];
-            property->setValue(value);
-            list->append(property);
-        }
-    }
-    return new CSSStyleDeclarationImpl(0, list);
-}
-
-void CSSComputedStyleDeclarationImpl::diff(CSSStyleDeclarationImpl *style) const
-{
-    if (!style)
-        return;
-
-    QValueList<int> properties;
-    for (QPtrListIterator<CSSProperty> it(*style->values()); it.current(); ++it) {
-        CSSProperty *property = it.current();
-        CSSValueImpl *value = getPropertyCSSValue(property->id());
-        if (value && value->cssText() == property->value()->cssText()) {
-            properties.append(property->id());
-        }
-    }
-    
-    for (QValueListIterator<int> it(properties.begin()); it != properties.end(); ++it)
-        style->removeProperty(*it);
+    return copyPropertiesInSet(InheritableProperties, sizeof(InheritableProperties) / sizeof(InheritableProperties[0]));
 }
 
 } // namespace DOM
index 3189132..aa50f75 100644 (file)
@@ -62,7 +62,6 @@ public:
     virtual DOMString item(unsigned long index) const;
 
     CSSStyleDeclarationImpl *copyInheritableProperties() const;
-    void diff(CSSStyleDeclarationImpl *) const;
 
     CSSValueImpl* getPositionOffsetValue(int propertyID) const;
 
index 4681cf1..2073d9d 100644 (file)
@@ -424,6 +424,58 @@ void CSSStyleDeclarationImpl::merge(CSSStyleDeclarationImpl *other, bool argOver
     }
 }
 
+void CSSStyleDeclarationImpl::diff(CSSStyleDeclarationImpl *style) const
+{
+    if (!style)
+        return;
+
+    QValueList<int> properties;
+    for (QPtrListIterator<CSSProperty> it(*style->values()); it.current(); ++it) {
+        CSSProperty *property = it.current();
+        CSSValueImpl *value = getPropertyCSSValue(property->id());
+        if (value && value->cssText() == property->value()->cssText()) {
+            properties.append(property->id());
+        }
+    }
+    
+    for (QValueListIterator<int> it(properties.begin()); it != properties.end(); ++it)
+        style->removeProperty(*it);
+}
+
+// This is the list of properties we want to copy in the copyBlockProperties() function.
+// It is the list of CSS properties that apply to specially to block-level elements.
+static const int BlockProperties[] = {
+    CSS_PROP_ORPHANS,
+    CSS_PROP_OVERFLOW, // This can be also be applied to replaced elements
+    CSS_PROP_PAGE_BREAK_AFTER,
+    CSS_PROP_PAGE_BREAK_BEFORE,
+    CSS_PROP_PAGE_BREAK_INSIDE,
+    CSS_PROP_TEXT_ALIGN,
+    CSS_PROP_TEXT_INDENT,
+    CSS_PROP_WIDOWS
+};
+
+CSSStyleDeclarationImpl *CSSStyleDeclarationImpl::copyBlockProperties() const
+{
+    return copyPropertiesInSet(BlockProperties, sizeof(BlockProperties) / sizeof(BlockProperties[0]));
+}
+
+CSSStyleDeclarationImpl *CSSStyleDeclarationImpl::copyPropertiesInSet(const int *set, unsigned length) const
+{
+    QPtrList<CSSProperty> *list = new QPtrList<CSSProperty>;
+    list->setAutoDelete(true);
+    for (unsigned i = 0; i < length; i++) {
+        CSSValueImpl *value = getPropertyCSSValue(set[i]);
+        if (value) {
+            CSSProperty *property = new CSSProperty;
+            property->m_id = set[i];
+            property->setValue(value);
+            list->append(property);
+        }
+    }
+    return new CSSStyleDeclarationImpl(0, list);
+}
+
 // --------------------------------------------------------------------------------------
 
 CSSValueImpl::CSSValueImpl()
index c8ee9e4..335ed39 100644 (file)
@@ -86,6 +86,9 @@ public:
     NodeImpl* node() const { return m_node; }
     
     void merge(CSSStyleDeclarationImpl *, bool argOverridesOnConflict=true);
+    void diff(CSSStyleDeclarationImpl *) const;
+
+    CSSStyleDeclarationImpl *copyBlockProperties() const;
 
     void setChanged();
 
@@ -93,6 +96,8 @@ protected:
     DOMString getShortHandValue( const int* properties, int number ) const;
     DOMString get4Values( const int* properties ) const;
 
+    CSSStyleDeclarationImpl *copyPropertiesInSet(const int *set, unsigned length) const;
+
     QPtrList<CSSProperty> *m_lstValues;
     NodeImpl *m_node;
 
index 03db138..3e283d1 100644 (file)
@@ -308,21 +308,20 @@ EditCommandPtr &EditCommandPtr::emptyCommand()
 //------------------------------------------------------------------------------------------
 // StyleChange
 
-StyleChange::StyleChange(CSSStyleDeclarationImpl *style) 
+StyleChange::StyleChange(CSSStyleDeclarationImpl *style, ELegacyHTMLStyles usesLegacyStyles)
+    : m_applyBold(false), m_applyItalic(false), m_usesLegacyStyles(usesLegacyStyles)
 {
     init(style, Position());
 }
 
-StyleChange::StyleChange(CSSStyleDeclarationImpl *style, const Position &position)
+StyleChange::StyleChange(CSSStyleDeclarationImpl *style, const Position &position, ELegacyHTMLStyles usesLegacyStyles)
+    : m_applyBold(false), m_applyItalic(false), m_usesLegacyStyles(usesLegacyStyles)
 {
     init(style, position);
 }
 
 void StyleChange::init(CSSStyleDeclarationImpl *style, const Position &position)
 {
-    m_applyBold = false;
-    m_applyItalic = false;
-
     QString styleText;
 
     for (QPtrListIterator<CSSProperty> it(*(style->values())); it.current(); ++it) {
@@ -332,23 +331,10 @@ void StyleChange::init(CSSStyleDeclarationImpl *style, const Position &position)
         // style, just move on.
         if (position.isNotNull() && currentlyHasStyle(position, property))
             continue;
-
-        // Figure out the manner of change that is needed.
-        DOMString valueText(property->value()->cssText());
-        switch (property->id()) {
-            case CSS_PROP_FONT_WEIGHT:
-                if (strcasecmp(valueText, "bold") == 0) {
-                    m_applyBold = true;
-                    continue;
-                }
-                break;
-            case CSS_PROP_FONT_STYLE:
-                if (strcasecmp(valueText, "italic") == 0 || strcasecmp(valueText, "oblique") == 0) {
-                    m_applyItalic = true;
-                    continue;
-                }
-                break;
-        }
+        
+        // If needed, figure out if this change is a legacy HTML style change.
+        if (m_usesLegacyStyles && checkForLegacyHTMLStyleChange(property))
+            continue;
 
         styleText += property->cssText().string();
     }
@@ -356,6 +342,26 @@ void StyleChange::init(CSSStyleDeclarationImpl *style, const Position &position)
     m_cssStyle = styleText.stripWhiteSpace();
 }
 
+bool StyleChange::checkForLegacyHTMLStyleChange(const DOM::CSSProperty *property)
+{
+    DOMString valueText(property->value()->cssText());
+    switch (property->id()) {
+        case CSS_PROP_FONT_WEIGHT:
+            if (strcasecmp(valueText, "bold") == 0) {
+                m_applyBold = true;
+                return true;
+            }
+            break;
+        case CSS_PROP_FONT_STYLE:
+            if (strcasecmp(valueText, "italic") == 0 || strcasecmp(valueText, "oblique") == 0) {
+                m_applyItalic = true;
+                return true;
+            }
+            break;
+    }
+    return false;
+}
+
 bool StyleChange::currentlyHasStyle(const Position &pos, const CSSProperty *property)
 {
     ASSERT(pos.isNotNull());
@@ -961,9 +967,58 @@ ApplyStyleCommand::~ApplyStyleCommand()
 
 void ApplyStyleCommand::doApply()
 {
-    if (!endingSelection().isRange())
-        return;
+    CSSStyleDeclarationImpl *blockStyle = m_style->copyBlockProperties();
+    blockStyle->ref();
+    applyBlockStyle(blockStyle);
+
+    if (blockStyle->length() < m_style->length()) {
+        CSSStyleDeclarationImpl *inlineStyle = new CSSStyleDeclarationImpl(0, m_style->values());
+        inlineStyle->ref();
+        blockStyle->diff(inlineStyle);
+        applyInlineStyle(inlineStyle);
+        inlineStyle->deref();
+    }
+
+    blockStyle->deref();
+    
+    setEndingSelectionNeedsLayout();
+}
+
+void ApplyStyleCommand::applyBlockStyle(CSSStyleDeclarationImpl *style)
+{
+    // update document layout once before removing styles
+    // so that we avoid the expense of updating before each and every call
+    // to check a computed style
+    document()->updateLayout();
 
+    // get positions we want to use for applying style
+    Position start(endingSelection().start());
+    Position end(endingSelection().end());
+    
+    // Remove block styles
+    NodeImpl *beyondEnd = end.node()->traverseNextNode();
+    NodeImpl *prevBlock = 0;
+    for (NodeImpl *node = start.node(); node != beyondEnd; node = node->traverseNextNode()) {
+        NodeImpl *block = node->enclosingBlockFlowElement();
+        if (block != prevBlock && block->isHTMLElement()) {
+            removeCSSStyle(style, static_cast<HTMLElementImpl *>(block));
+            prevBlock = block;
+        }
+    }
+    
+    // Apply new styles
+    prevBlock = 0;
+    for (NodeImpl *node = start.node(); node != beyondEnd; node = node->traverseNextNode()) {
+        NodeImpl *block = node->enclosingBlockFlowElement();
+        if (block != prevBlock && block->isHTMLElement()) {
+            addBlockStyleIfNeeded(style, static_cast<HTMLElementImpl *>(block));
+            prevBlock = block;
+        }
+    }
+}
+
+void ApplyStyleCommand::applyInlineStyle(CSSStyleDeclarationImpl *style)
+{
     // adjust to the positions we want to use for applying style
     Position start(endingSelection().start().downstream(StayInBlock).equivalentRangeCompliantPosition());
     Position end(endingSelection().end().upstream(StayInBlock));
@@ -978,7 +1033,7 @@ void ApplyStyleCommand::doApply()
     // This will ensure we remove all traces of the relevant styles from the selection
     // and prevent us from adding redundant ones, as described in:
     // <rdar://problem/3724344> Bolding and unbolding creates extraneous tags
-    removeStyle(start.upstream(), end);
+    removeInlineStyle(style, start.upstream(), end);
     
     bool splitStart = splitTextAtStartIfNeeded(start, end); 
     if (splitStart) {
@@ -996,7 +1051,7 @@ void ApplyStyleCommand::doApply()
     
     if (start.node() == end.node()) {
         // simple case...start and end are the same node
-        applyStyleIfNeeded(start.node(), end.node());
+        addInlineStyleIfNeeded(style, start.node(), end.node());
     }
     else {
         NodeImpl *node = start.node();
@@ -1015,7 +1070,7 @@ void ApplyStyleCommand::doApply()
                     node = next;
                 }
                 // Now apply style to the run we found.
-                applyStyleIfNeeded(runStart, node);
+                addInlineStyleIfNeeded(style, runStart, node);
             }
             if (node == end.node())
                 break;
@@ -1027,9 +1082,9 @@ void ApplyStyleCommand::doApply()
 //------------------------------------------------------------------------------------------
 // ApplyStyleCommand: style-removal helpers
 
-bool ApplyStyleCommand::isHTMLStyleNode(HTMLElementImpl *elem)
+bool ApplyStyleCommand::isHTMLStyleNode(CSSStyleDeclarationImpl *style, HTMLElementImpl *elem)
 {
-    for (QPtrListIterator<CSSProperty> it(*(style()->values())); it.current(); ++it) {
+    for (QPtrListIterator<CSSProperty> it(*(style->values())); it.current(); ++it) {
         CSSProperty *property = it.current();
         switch (property->id()) {
             case CSS_PROP_FONT_WEIGHT:
@@ -1056,21 +1111,22 @@ void ApplyStyleCommand::removeHTMLStyleNode(HTMLElementImpl *elem)
     removeNodePreservingChildren(elem);
 }
 
-void ApplyStyleCommand::removeCSSStyle(HTMLElementImpl *elem)
+void ApplyStyleCommand::removeCSSStyle(CSSStyleDeclarationImpl *style, HTMLElementImpl *elem)
 {
+    ASSERT(style);
     ASSERT(elem);
 
     CSSStyleDeclarationImpl *decl = elem->inlineStyleDecl();
     if (!decl)
         return;
 
-    for (QPtrListIterator<CSSProperty> it(*(style()->values())); it.current(); ++it) {
+    for (QPtrListIterator<CSSProperty> it(*(style->values())); it.current(); ++it) {
         CSSProperty *property = it.current();
         if (decl->getPropertyCSSValue(property->id()))
             removeCSSProperty(decl, property->id());
     }
 
-    if (elem->id() == ID_SPAN) {
+    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.
@@ -1083,22 +1139,33 @@ void ApplyStyleCommand::removeCSSStyle(HTMLElementImpl *elem)
     }
 }
 
-void ApplyStyleCommand::removeStyle(const Position &start, const Position &end)
+void ApplyStyleCommand::removeBlockStyle(CSSStyleDeclarationImpl *style, const Position &start, const Position &end)
+{
+    ASSERT(start.isNotNull());
+    ASSERT(end.isNotNull());
+    ASSERT(start.node()->inDocument());
+    ASSERT(end.node()->inDocument());
+    ASSERT(RangeImpl::compareBoundaryPoints(start.node(), start.offset(), end.node(), end.offset()) <= 0);
+    
+}
+
+void ApplyStyleCommand::removeInlineStyle(CSSStyleDeclarationImpl *style, const Position &start, const Position &end)
 {
     ASSERT(start.isNotNull());
     ASSERT(end.isNotNull());
     ASSERT(start.node()->inDocument());
     ASSERT(end.node()->inDocument());
+    ASSERT(RangeImpl::compareBoundaryPoints(start.node(), start.offset(), end.node(), end.offset()) <= 0);
     
     NodeImpl *node = start.node();
     while (node) {
         NodeImpl *next = node->traverseNextNode();
         if (node->isHTMLElement() && nodeFullySelected(node, start, end)) {
             HTMLElementImpl *elem = static_cast<HTMLElementImpl *>(node);
-            if (isHTMLStyleNode(elem))
+            if (isHTMLStyleNode(style, elem))
                 removeHTMLStyleNode(elem);
             else
-                removeCSSStyle(elem);
+                removeCSSStyle(style, elem);
         }
         if (node == end.node())
             break;
@@ -1166,12 +1233,26 @@ void ApplyStyleCommand::surroundNodeRangeWithElement(NodeImpl *startNode, NodeIm
     }
 }
 
-void ApplyStyleCommand::applyStyleIfNeeded(NodeImpl *startNode, NodeImpl *endNode)
+void ApplyStyleCommand::addBlockStyleIfNeeded(CSSStyleDeclarationImpl *style, HTMLElementImpl *block)
+{
+    // Do not check for legacy styles here. Those styles, like <B> and <I>, only apply for
+    // inline content.
+    StyleChange styleChange(style, Position(block, 0), StyleChange::DoNotUseLegacyHTMLStyles);
+    if (styleChange.cssStyle().length() > 0) {
+        DOMString cssText = styleChange.cssStyle();
+        CSSStyleDeclarationImpl *decl = block->inlineStyleDecl();
+        if (decl)
+            cssText += decl->cssText();
+        block->setAttribute(ATTR_STYLE, cssText);
+    }
+}
+
+void ApplyStyleCommand::addInlineStyleIfNeeded(CSSStyleDeclarationImpl *style, NodeImpl *startNode, NodeImpl *endNode)
 {
     // FIXME: This function should share code with CompositeEditCommand::applyTypingStyle.
     // Both functions do similar work, and the common parts could be factored out.
 
-    StyleChange styleChange(style(), Position(startNode, 0));
+    StyleChange styleChange(style, Position(startNode, 0));
     int exceptionCode = 0;
     
     if (styleChange.cssStyle().length() > 0) {
index 916d0b3..b488e28 100644 (file)
@@ -90,21 +90,25 @@ public:
 
 class StyleChange {
 public:
-    StyleChange() : m_applyBold(false), m_applyItalic(false) { }
-    explicit StyleChange(DOM::CSSStyleDeclarationImpl *);
-    StyleChange(DOM::CSSStyleDeclarationImpl *, const DOM::Position &);
+    enum ELegacyHTMLStyles { DoNotUseLegacyHTMLStyles, UseLegacyHTMLStyles };
+
+    explicit StyleChange(DOM::CSSStyleDeclarationImpl *, ELegacyHTMLStyles usesLegacyStyles=UseLegacyHTMLStyles);
+    StyleChange(DOM::CSSStyleDeclarationImpl *, const DOM::Position &, ELegacyHTMLStyles usesLegacyStyles=UseLegacyHTMLStyles);
 
     DOM::DOMString cssStyle() const { return m_cssStyle; }
     bool applyBold() const { return m_applyBold; }
     bool applyItalic() const { return m_applyItalic; }
+    bool usesLegacyStyles() const { return m_usesLegacyStyles; }
 
 private:
     void init(DOM::CSSStyleDeclarationImpl *, const DOM::Position &);
+    bool checkForLegacyHTMLStyleChange(const DOM::CSSProperty *);
     static bool currentlyHasStyle(const DOM::Position &, const DOM::CSSProperty *);
     
     DOM::DOMString m_cssStyle;
     bool m_applyBold;
     bool m_applyItalic;
+    bool m_usesLegacyStyles;
 };
 
 //------------------------------------------------------------------------------------------
@@ -134,6 +138,8 @@ public:
 
     khtml::Selection startingSelection() const { return m_startingSelection; }
     khtml::Selection endingSelection() const { return m_endingSelection; }
+
+    void setEndingSelectionNeedsLayout(bool flag=true) { m_endingSelection.setNeedsLayout(flag); }
         
     ECommandState state() const { return m_state; }
     void setState(ECommandState state) { m_state = state; }
@@ -245,18 +251,22 @@ public:
 
 private:
     // style-removal helpers
-    bool isHTMLStyleNode(DOM::HTMLElementImpl *);
+    bool isHTMLStyleNode(DOM::CSSStyleDeclarationImpl *, DOM::HTMLElementImpl *);
     void removeHTMLStyleNode(DOM::HTMLElementImpl *);
-    void removeCSSStyle(DOM::HTMLElementImpl *);
-    void removeStyle(const DOM::Position &start, const DOM::Position &end);
+    void removeCSSStyle(DOM::CSSStyleDeclarationImpl *, DOM::HTMLElementImpl *);
+    void removeBlockStyle(DOM::CSSStyleDeclarationImpl *, const DOM::Position &start, const DOM::Position &end);
+    void removeInlineStyle(DOM::CSSStyleDeclarationImpl *, const DOM::Position &start, const DOM::Position &end);
     bool nodeFullySelected(DOM::NodeImpl *, const DOM::Position &start, const DOM::Position &end) const;
 
     // style-application helpers
+    void applyBlockStyle(DOM::CSSStyleDeclarationImpl *);
+    void applyInlineStyle(DOM::CSSStyleDeclarationImpl *);
+    void addBlockStyleIfNeeded(DOM::CSSStyleDeclarationImpl *, DOM::HTMLElementImpl *);
+    void addInlineStyleIfNeeded(DOM::CSSStyleDeclarationImpl *, DOM::NodeImpl *start, DOM::NodeImpl *end);
     bool splitTextAtStartIfNeeded(const DOM::Position &start, const DOM::Position &end);
     DOM::NodeImpl *splitTextAtEndIfNeeded(const DOM::Position &start, const DOM::Position &end);
     void surroundNodeRangeWithElement(DOM::NodeImpl *start, DOM::NodeImpl *end, DOM::ElementImpl *element);
     DOM::Position positionInsertionPoint(DOM::Position);
-    void applyStyleIfNeeded(DOM::NodeImpl *start, DOM::NodeImpl *end);
     
     DOM::CSSStyleDeclarationImpl *m_style;
 };
index fd70a44..3a41d31 100644 (file)
@@ -5033,6 +5033,9 @@ CSSStyleDeclarationImpl *KHTMLPart::typingStyle() const
 
 void KHTMLPart::setTypingStyle(CSSStyleDeclarationImpl *style)
 {
+    if (d->m_typingStyle == style)
+        return;
+        
     CSSStyleDeclarationImpl *old = d->m_typingStyle;
     d->m_typingStyle = style;
     if (d->m_typingStyle)
@@ -5317,13 +5320,28 @@ void KHTMLPart::applyStyle(CSSStyleDeclarationImpl *style)
             // do nothing
             break;
         case Selection::CARET: {
+            // Calculate the current typing style.
             if (typingStyle()) {
                 typingStyle()->merge(style);
                 style = typingStyle();
             }
-            CSSComputedStyleDeclarationImpl diff(selection().start().upstream(StayInBlock).node());
-            diff.diff(style);
+            style->ref();
+            CSSComputedStyleDeclarationImpl computedStyle(selection().start().upstream(StayInBlock).node());
+            computedStyle.diff(style);
+            
+            // Handle block styles, substracting these from the typing style.
+            CSSStyleDeclarationImpl *blockStyle = style->copyBlockProperties();
+            blockStyle->ref();
+            blockStyle->diff(style);
+            if (xmlDocImpl() && blockStyle->length() > 0) {
+                EditCommandPtr cmd(new ApplyStyleCommand(xmlDocImpl(), blockStyle));
+                cmd.apply();
+            }
+            blockStyle->deref();
+            
+            // Set the remaining style as the typing style.
             setTypingStyle(style);
+            style->deref();
             break;
         }
         case Selection::RANGE: