Changeset 24792 in webkit

Timestamp:

Jul 30, 2007 8:54:56 PM (17 years ago)

Author:

justing

Message:

LayoutTests:

Reviewed by Darin.

<rdar://problem/5369009>
Crash due to infinite recursion in moveParagraphs deleting line from a Mail message

• editing/deleting/5369009-expected.checksum: Added.
• editing/deleting/5369009-expected.png: Added.
• editing/deleting/5369009-expected.txt: Added.
• editing/deleting/5369009.html: Added.

WebCore:

Reviewed by Darin.

<rdar://problem/5369009> Crash due to infinite recursion in moveParagraphs on delete

After the delete, we moved content into the previous block, and a style
rule turned a style span in the moved content into a block, throwing
moveParagraphs into infinite recursion, as it continually tried and failed
to get the style span into the same paragraph as the content just before it.

Added a method to ReplaceSelectionCommand to keep inserted style spans
from turning into blocks because of style rules. Will add code to prevent
other kinds of content from changing appearance because of style rules,
post-Leopard (5371536). Also added a bool to ReplaceSelectionCommand's
constructor to guard against infinite recursion.

• editing/CompositeEditCommand.cpp: (WebCore::CompositeEditCommand::moveParagraphs):
• editing/JSEditor.cpp:
• editing/ReplaceSelectionCommand.cpp: (WebCore::ReplaceSelectionCommand::ReplaceSelectionCommand): (WebCore::ReplaceSelectionCommand::negateStyleRulesThatEffectAppearance): (WebCore::ReplaceSelectionCommand::doApply):
• editing/ReplaceSelectionCommand.h:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/deleting/5369009-expected.checksum (added)
• LayoutTests/editing/deleting/5369009-expected.png (added)
• LayoutTests/editing/deleting/5369009-expected.txt (added)
• LayoutTests/editing/deleting/5369009.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/editing/CompositeEditCommand.cpp (modified) (1 diff)
• WebCore/editing/JSEditor.cpp (modified) (1 diff)
• WebCore/editing/ReplaceSelectionCommand.cpp (modified) (7 diffs)
• WebCore/editing/ReplaceSelectionCommand.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2007-07-30  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin.
+        
+        <rdar://problem/5369009>
+        Crash due to infinite recursion in moveParagraphs deleting line from a Mail message
+
+        * editing/deleting/5369009-expected.checksum: Added.
+        * editing/deleting/5369009-expected.png: Added.
+        * editing/deleting/5369009-expected.txt: Added.
+        * editing/deleting/5369009.html: Added.
+
 2007-07-30  Sam Weinig  <sam@webkit.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-07-30  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Darin.
+
+        <rdar://problem/5369009> Crash due to infinite recursion in moveParagraphs on delete
+        
+        After the delete, we moved content into the previous block, and a style
+        rule turned a style span in the moved content into a block, throwing
+        moveParagraphs into infinite recursion, as it continually tried and failed
+        to get the style span into the same paragraph as the content just before it.
+        
+        Added a method to ReplaceSelectionCommand to keep inserted style spans 
+        from turning into blocks because of style rules.  Will add code to prevent
+        other kinds of content from changing appearance because of style rules,
+        post-Leopard (5371536). Also added a bool to ReplaceSelectionCommand's 
+        constructor to guard against infinite recursion.
+
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::moveParagraphs):
+        * editing/JSEditor.cpp:
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::ReplaceSelectionCommand):
+        (WebCore::ReplaceSelectionCommand::negateStyleRulesThatEffectAppearance):
+        (WebCore::ReplaceSelectionCommand::doApply):
+        * editing/ReplaceSelectionCommand.h:
+
 2007-07-30  Anders Carlsson  <andersca@apple.com>
 

trunk/WebCore/editing/CompositeEditCommand.cpp

@@ -784 +784 @@
     
     setEndingSelection(destination);
-    applyCommandToComposite(new ReplaceSelectionCommand(document(), fragment.get(), true, false, !preserveStyle, false));
+    applyCommandToComposite(new ReplaceSelectionCommand(document(), fragment.get(), true, false, !preserveStyle, false, true));
     
     if (preserveSelection && startIndex != -1) {

trunk/WebCore/editing/JSEditor.cpp

@@ -295 +295 @@
     
     applyCommand(new ReplaceSelectionCommand(frame->document(), fragment.release(),
-        false, false, false, true, EditActionUnspecified));
+        false, false, false, true, false, EditActionUnspecified));
     return true;
 }

trunk/WebCore/editing/ReplaceSelectionCommand.cpp

@@ -31 +31 @@
 #include "CSSComputedStyleDeclaration.h"
 #include "CSSPropertyNames.h"
+#include "CSSValueKeywords.h"
 #include "Document.h"
 #include "DocumentFragment.h"
@@ -275 +276 @@
 
 ReplaceSelectionCommand::ReplaceSelectionCommand(Document* document, PassRefPtr<DocumentFragment> fragment,
-        bool selectReplacement, bool smartReplace, bool matchStyle, bool preventNesting,
+        bool selectReplacement, bool smartReplace, bool matchStyle, bool preventNesting, bool movingParagraph,
         EditAction editAction) 
     : CompositeEditCommand(document),
@@ -283 +284 @@
       m_documentFragment(fragment),
       m_preventNesting(preventNesting),
+      m_movingParagraph(movingParagraph),
       m_editAction(editAction)
 {
@@ -344 +346 @@
            // be a no-op and cause infinite recursion.
            !isBlock(fromNode) && !isBlock(toNode);
+}
+
+// Style rules that match just inserted elements could change their appearance, like
+// a div inserted into a document with div { display:inline; }.
+void ReplaceSelectionCommand::negateStyleRulesThatAffectAppearance()
+{
+    for (RefPtr<Node> node = m_firstNodeInserted.get(); node; node = node->traverseNextNode()) {
+        // FIXME: <rdar://problem/5371536> Style rules that match pasted content can change it's appearance
+        if (isStyleSpan(node.get())) {
+            HTMLElement* e = static_cast<HTMLElement*>(node.get());
+            // There are other styles that style rules can give to style spans,
+            // but these are the two important ones because they'll prevent
+            // inserted content from appearing in the right paragraph.
+            if (isBlock(e))
+                e->getInlineStyleDecl()->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE);
+            if (e->renderer() && e->renderer()->style()->floating() != FNONE)
+                e->getInlineStyleDecl()->setProperty(CSS_PROP_FLOAT, CSS_VAL_NONE);
+        }
+        if (node == m_lastLeafInserted)
+            break;
+    }
 }
 
@@ -585 +608 @@
     }
     
+    negateStyleRulesThatAffectAppearance();
+    
     removeRedundantStyles(mailBlockquoteEnclosingSelectionStart);
     
@@ -603 +628 @@
         
     if (shouldMergeStart(selectionStartWasStartOfParagraph, fragment.hasInterchangeNewlineAtStart())) {
+        // Bail to avoid infinite recursion.
+        if (m_movingParagraph) {
+            ASSERT_NOT_REACHED();
+            return;
+        }
         VisiblePosition destination = startOfInsertedContent.previous();
         VisiblePosition startOfParagraphToMove = startOfInsertedContent;
@@ -643 +673 @@
             
     } else if (shouldMergeEnd(selectionEndWasEndOfParagraph)) {
-    
+        // Bail to avoid infinite recursion.
+        if (m_movingParagraph) {
+            ASSERT_NOT_REACHED();
+            return;
+        }
         // Merging two paragraphs will destroy the moved one's block styles.  Always move forward to preserve
         // the block style of the paragraph already in the document, unless the paragraph to move would include the

trunk/WebCore/editing/ReplaceSelectionCommand.h

@@ -70 +70 @@
 public:
     ReplaceSelectionCommand(Document*, PassRefPtr<DocumentFragment>,
-        bool selectReplacement = true, bool smartReplace = false, bool matchStyle = false, bool preventNesting = true,
+        bool selectReplacement = true, bool smartReplace = false, bool matchStyle = false, bool preventNesting = true, bool movingParagraph = false,
         EditAction = EditActionPaste);
     
@@ -90 +90 @@
     bool shouldMerge(const VisiblePosition&, const VisiblePosition&);
     
+    void negateStyleRulesThatAffectAppearance();
     void removeRedundantStyles(Node*);
     
@@ -107 +108 @@
     RefPtr<DocumentFragment> m_documentFragment;
     bool m_preventNesting;
+    bool m_movingParagraph;
     EditAction m_editAction;
 };