Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jan 2005 21:21:08 +0000 (21:21 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Jan 2005 21:21:08 +0000 (21:21 +0000)
        Fix for this bug:

        <rdar://problem/3954710> Mail crashed while editing signatures - NodeImpl::isBlockFlow

        The fix is more general than for this one bug, and may work to fix many crashers. The problem
        is that the ReplaceSelectionCommand never checked whether its starting selection is empty. If
        it is, then we need to bail before doing the work of the command, which we need to deref the
        start and end points of the selection in order to do its work. I think you can see the crash
        potential.

        * khtml/editing/htmlediting.cpp:
        (khtml::ReplaceSelectionCommand::doApply): Assert selection is not empty.
        * kwq/WebCoreBridge.mm:
        (partHasSelection): New helper function to test that bridge has a part with a selection.

        Use new helper function to test part and selection; return from these function if this test fails.

        (-[WebCoreBridge rangeByExpandingSelectionWithGranularity:])
        (-[WebCoreBridge rangeByAlteringCurrentSelection:direction:granularity:])
        (-[WebCoreBridge alterCurrentSelection:direction:granularity:])
        (-[WebCoreBridge rangeByAlteringCurrentSelection:verticalDistance:])
        (-[WebCoreBridge alterCurrentSelection:verticalDistance:])
        (-[WebCoreBridge documentFragmentWithText:])
        (-[WebCoreBridge replaceSelectionWithFragment:selectReplacement:smartReplace:])
        (-[WebCoreBridge insertLineBreak])
        (-[WebCoreBridge insertParagraphSeparator])
        (-[WebCoreBridge insertParagraphSeparatorInQuotedContent])
        (-[WebCoreBridge insertText:selectInsertedText:])
        (-[WebCoreBridge deleteSelectionWithSmartDelete:])
        (-[WebCoreBridge ensureSelectionVisible])

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp
WebCore/kwq/WebCoreBridge.mm

index 8da9a00..b629b55 100644 (file)
@@ -1,3 +1,38 @@
+2005-01-25  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/3954710> Mail crashed while editing signatures - NodeImpl::isBlockFlow
+
+        The fix is more general than for this one bug, and may work to fix many crashers. The problem
+        is that the ReplaceSelectionCommand never checked whether its starting selection is empty. If
+        it is, then we need to bail before doing the work of the command, which we need to deref the
+        start and end points of the selection in order to do its work. I think you can see the crash
+        potential.
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::ReplaceSelectionCommand::doApply): Assert selection is not empty.
+        * kwq/WebCoreBridge.mm:
+        (partHasSelection): New helper function to test that bridge has a part with a selection.
+        
+        Use new helper function to test part and selection; return from these function if this test fails.
+        
+        (-[WebCoreBridge rangeByExpandingSelectionWithGranularity:])
+        (-[WebCoreBridge rangeByAlteringCurrentSelection:direction:granularity:])
+        (-[WebCoreBridge alterCurrentSelection:direction:granularity:])
+        (-[WebCoreBridge rangeByAlteringCurrentSelection:verticalDistance:])
+        (-[WebCoreBridge alterCurrentSelection:verticalDistance:])
+        (-[WebCoreBridge documentFragmentWithText:])
+        (-[WebCoreBridge replaceSelectionWithFragment:selectReplacement:smartReplace:])
+        (-[WebCoreBridge insertLineBreak])
+        (-[WebCoreBridge insertParagraphSeparator])
+        (-[WebCoreBridge insertParagraphSeparatorInQuotedContent])
+        (-[WebCoreBridge insertText:selectInsertedText:])
+        (-[WebCoreBridge deleteSelectionWithSmartDelete:])
+        (-[WebCoreBridge ensureSelectionVisible])
+
 2005-01-24  Kevin Decker  <kdecker@apple.com>
 
         Reviewed by Darin.
index 493f85a..cf1e10e 100644 (file)
@@ -3907,6 +3907,7 @@ ReplaceSelectionCommand::~ReplaceSelectionCommand()
 void ReplaceSelectionCommand::doApply()
 {
     Selection selection = endingSelection();
+    ASSERT(selection.isCaretOrRange());
     VisiblePosition visibleStart(selection.start());
     VisiblePosition visibleEnd(selection.end());
     bool startAtStartOfBlock = isFirstVisiblePositionInBlock(visibleStart);
index da78c4b..028a7ba 100644 (file)
@@ -188,6 +188,23 @@ static void updateRenderingForBindings (ExecState *exec, ObjectImp *rootObject)
         doc->updateRendering();
 }
 
+static BOOL partHasSelection(WebCoreBridge *bridge)
+{
+    if (!bridge)
+        return NO;
+    
+    KHTMLPart *part = bridge->_part;
+    if (!part)
+        return NO;
+        
+    if (part->selection().isNone())
+        return NO;
+
+    // If a part has a selection, it should also have a document.        
+    ASSERT(part->xmlDocImpl());
+
+    return YES;
+}
 
 @implementation WebCoreBridge
 
@@ -1415,7 +1432,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (DOMRange *)rangeByExpandingSelectionWithGranularity:(WebSelectionGranularity)granularity
 {
-    if (!_part)
+    if (!partHasSelection(self))
         return nil;
         
     // NOTE: The enums *must* match the very similar ones declared in ktml_selection.h
@@ -1426,7 +1443,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (DOMRange *)rangeByAlteringCurrentSelection:(WebSelectionAlteration)alteration direction:(WebSelectionDirection)direction granularity:(WebSelectionGranularity)granularity
 {
-    if (!_part)
+    if (!partHasSelection(self))
         return nil;
         
     // NOTE: The enums *must* match the very similar ones declared in ktml_selection.h
@@ -1439,7 +1456,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)alterCurrentSelection:(WebSelectionAlteration)alteration direction:(WebSelectionDirection)direction granularity:(WebSelectionGranularity)granularity
 {
-    if (!_part)
+    if (!partHasSelection(self))
         return;
         
     // NOTE: The enums *must* match the very similar ones declared in dom_selection.h
@@ -1476,7 +1493,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (DOMRange *)rangeByAlteringCurrentSelection:(WebSelectionAlteration)alteration verticalDistance:(float)verticalDistance
 {
-    if (!_part)
+    if (!partHasSelection(self))
         return nil;
         
     Selection selection(_part->selection());
@@ -1486,7 +1503,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)alterCurrentSelection:(WebSelectionAlteration)alteration verticalDistance:(float)verticalDistance
 {
-    if (!_part)
+    if (!partHasSelection(self))
         return;
         
     Selection selection(_part->selection());
@@ -1569,15 +1586,15 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (DOMDocumentFragment *)documentFragmentWithText:(NSString *)text
 {
-    if (!_part || !_part->xmlDocImpl() || !text)
+    if (!partHasSelection(self) || !text)
         return 0;
-
+    
     return [DOMDocumentFragment _documentFragmentWithImpl:createFragmentFromText(_part->xmlDocImpl(), QString::fromNSString(text))];
 }
 
 - (void)replaceSelectionWithFragment:(DOMDocumentFragment *)fragment selectReplacement:(BOOL)selectReplacement smartReplace:(BOOL)smartReplace
 {
-    if (!_part || !_part->xmlDocImpl() || !fragment)
+    if (!partHasSelection(self) || !fragment)
         return;
     
     EditCommandPtr(new ReplaceSelectionCommand(_part->xmlDocImpl(), [fragment _fragmentImpl], selectReplacement, smartReplace)).apply();
@@ -1604,7 +1621,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)insertLineBreak
 {
-    if (!_part || !_part->xmlDocImpl())
+    if (!partHasSelection(self))
         return;
     
     TypingCommand::insertLineBreak(_part->xmlDocImpl());
@@ -1613,7 +1630,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)insertParagraphSeparator
 {
-    if (!_part || !_part->xmlDocImpl())
+    if (!partHasSelection(self))
         return;
     
     TypingCommand::insertParagraphSeparator(_part->xmlDocImpl());
@@ -1622,11 +1639,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)insertParagraphSeparatorInQuotedContent
 {
-    if (!_part || !_part->xmlDocImpl())
-        return;
-    
-    Selection selection(_part->selection());
-    if (selection.isNone())
+    if (!partHasSelection(self))
         return;
     
     TypingCommand::insertParagraphSeparatorInQuotedContent(_part->xmlDocImpl());
@@ -1635,7 +1648,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)insertText:(NSString *)text selectInsertedText:(BOOL)selectInsertedText
 {
-    if (!_part || !_part->xmlDocImpl())
+    if (!partHasSelection(self))
         return;
     
     TypingCommand::insertText(_part->xmlDocImpl(), text, selectInsertedText);
@@ -1692,11 +1705,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)deleteSelectionWithSmartDelete:(BOOL)smartDelete
 {
-    if (!_part || !_part->xmlDocImpl())
-        return;
-    
-    Selection selection(_part->selection());
-    if (!selection.isRange())
+    if (!partHasSelection(self))
         return;
     
     EditCommandPtr(new DeleteSelectionCommand(_part->xmlDocImpl(), smartDelete)).apply();
@@ -1785,7 +1794,7 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
 
 - (void)ensureSelectionVisible
 {
-    if (!_part || _part->selection().isNone())
+    if (!partHasSelection(self))
         return;
     
     KHTMLView *v = _part->view();