WebCore:
authorcblu <cblu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 20 Sep 2004 23:56:20 +0000 (23:56 +0000)
committercblu <cblu@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 20 Sep 2004 23:56:20 +0000 (23:56 +0000)
Fixed: <rdar://problem/3781290> REGRESSION (Mail): Crash in ReplaceSelectionCommandImpl attaching file to new message

        Reviewed by kocienda.

        * khtml/khtml_part.cpp:
        (KHTMLPart::setSelection): setFocusNodeIfNeeded now works on the current selection so call setFocusNodeIfNeeded after setting the selection
        (KHTMLPart::clearSelection): don't call setFocusNodeIfNeeded with the current selection
        (KHTMLPart::setCaretVisible): ditto
        (KHTMLPart::setFocusNodeIfNeeded): do nothing if the part isn't focused, work with the current selection
        * khtml/khtml_part.h:
        * khtml/khtmlpart_p.h:
        (KHTMLPartPrivate::KHTMLPartPrivate): added m_isFocused
        * kwq/KWQKHTMLPart.h:
        * kwq/KWQKHTMLPart.mm:
        (KWQKHTMLPart::KWQKHTMLPart): removed _displaysWithFocusAttributes this is replaced by m_isFocused
        (KWQKHTMLPart::setSelectionFromNone): new, code factored out from setDisplaysWithFocusAttributes
        (KWQKHTMLPart::setDisplaysWithFocusAttributes): call setSelectionFromNone
        (KWQKHTMLPart::displaysWithFocusAttributes): now returns m_isFocused
        * kwq/WebCoreBridge.h:
        * kwq/WebCoreBridge.mm:
        (-[WebCoreBridge setSelectionFromNone]): new

WebKit:

Fixed: <rdar://problem/3781290> REGRESSION (Mail): Crash in ReplaceSelectionCommandImpl attaching file to new message

        Reviewed by kocienda.

        * WebView.subproj/WebView.m:
        (-[WebView setEditable:]): call updateSelectionFromEmpty on the bridge if there is no selection

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/khtml_part.cpp
WebCore/khtml/khtml_part.h
WebCore/khtml/khtmlpart_p.h
WebCore/kwq/KWQKHTMLPart.h
WebCore/kwq/KWQKHTMLPart.mm
WebCore/kwq/WebCoreBridge.h
WebCore/kwq/WebCoreBridge.mm
WebKit/ChangeLog
WebKit/WebView.subproj/WebView.m

index 4ea3b59673f23d7a66df2f9ac38a06c47078d306..82c28ac5471593816ad4460d5cf8f7952aebacd1 100644 (file)
@@ -1,3 +1,27 @@
+2004-09-20  Chris Blumenberg  <cblu@apple.com>
+
+       Fixed: <rdar://problem/3781290> REGRESSION (Mail): Crash in ReplaceSelectionCommandImpl attaching file to new message
+
+        Reviewed by kocienda.
+
+        * khtml/khtml_part.cpp:
+        (KHTMLPart::setSelection): setFocusNodeIfNeeded now works on the current selection so call setFocusNodeIfNeeded after setting the selection
+        (KHTMLPart::clearSelection): don't call setFocusNodeIfNeeded with the current selection
+        (KHTMLPart::setCaretVisible): ditto
+        (KHTMLPart::setFocusNodeIfNeeded): do nothing if the part isn't focused, work with the current selection
+        * khtml/khtml_part.h:
+        * khtml/khtmlpart_p.h:
+        (KHTMLPartPrivate::KHTMLPartPrivate): added m_isFocused
+        * kwq/KWQKHTMLPart.h:
+        * kwq/KWQKHTMLPart.mm:
+        (KWQKHTMLPart::KWQKHTMLPart): removed _displaysWithFocusAttributes this is replaced by m_isFocused
+        (KWQKHTMLPart::setSelectionFromNone): new, code factored out from setDisplaysWithFocusAttributes
+        (KWQKHTMLPart::setDisplaysWithFocusAttributes): call setSelectionFromNone
+        (KWQKHTMLPart::displaysWithFocusAttributes): now returns m_isFocused
+        * kwq/WebCoreBridge.h:
+        * kwq/WebCoreBridge.mm:
+        (-[WebCoreBridge setSelectionFromNone]): new
+
 2004-09-20  Darin Adler  <darin@apple.com>
 
         Reviewed by Dave.
index 10f34052c5e473ce0a86220a317e4848c8119b04..093a3f3e79cf8a6f235839567c7ba8c04c4a7d82 100644 (file)
@@ -2341,7 +2341,6 @@ void KHTMLPart::setSelection(const Selection &s, bool closeTyping)
 {
     if (d->m_selection != s) {
         clearCaretRectIfNeeded(); 
-        setFocusNodeIfNeeded(s);
 #if APPLE_CHANGES
         // Mark misspellings in the soon-to-be previous selection.  When typing we check spelling
         // elsewhere, so don't redo it here
@@ -2350,6 +2349,7 @@ void KHTMLPart::setSelection(const Selection &s, bool closeTyping)
         }
 #endif
         d->m_selection = s;
+        setFocusNodeIfNeeded();
         notifySelectionChanged(closeTyping);
     }
 }
@@ -2366,7 +2366,7 @@ void KHTMLPart::setDragCaret(const DOM::Selection &dragCaret)
 void KHTMLPart::clearSelection()
 {
     clearCaretRectIfNeeded();
-    setFocusNodeIfNeeded(d->m_selection);
+    setFocusNodeIfNeeded();
     d->m_selection = Selection();
     notifySelectionChanged();
 }
@@ -2384,7 +2384,7 @@ void KHTMLPart::setCaretVisible(bool flag)
         return;
 
     clearCaretRectIfNeeded();
-    setFocusNodeIfNeeded(d->m_selection);
+    setFocusNodeIfNeeded();
     d->m_caretVisible = flag;
     selectionLayoutChanged();
 }
@@ -2406,15 +2406,15 @@ void KHTMLPart::clearCaretRectIfNeeded()
     }        
 }
 
-void KHTMLPart::setFocusNodeIfNeeded(const Selection &s)
+void KHTMLPart::setFocusNodeIfNeeded()
 {
-    if (!xmlDocImpl() || s.state() == Selection::NONE)
+    if (!xmlDocImpl() || d->m_selection.state() == Selection::NONE || !d->m_isFocused)
         return;
 
-    NodeImpl *n = s.start().node();
+    NodeImpl *n = d->m_selection.start().node();
     NodeImpl *target = (n && n->isContentEditable()) ? n : 0;
     if (!target) {
-        while (n && n != s.end().node()) {
+        while (n && n != d->m_selection.end().node()) {
             if (n->isContentEditable()) {
                 target = n;
                 break;
index 4b5bac4c8c6cce5a273395cd149f83cf451daf03..d8781c47e596f3805d12b675b6a57e8727229c3b 100644 (file)
@@ -1185,7 +1185,7 @@ private:
   /**
    * @internal
    */
-  void setFocusNodeIfNeeded(const DOM::Selection &);
+  void setFocusNodeIfNeeded();
 
   /**
    * @internal
index 37bdbb5d980a9f72efc03dac754a6861249fdb1a..04657675d7bbd96448c3e0a06b03eef7b89221ee 100644 (file)
@@ -196,6 +196,7 @@ public:
         }
     }
 
+    m_isFocused = false;
     m_focusNodeNumber = -1;
     m_focusNodeRestored = false;
     m_opener = 0;
@@ -372,6 +373,7 @@ public:
   bool m_bCleared:1;
   bool m_bSecurityInQuestion:1;
   bool m_focusNodeRestored:1;
+  bool m_isFocused:1;
 
   khtml::EditCommand m_lastEditCommand;
   int m_xPosForVerticalArrowNavigation;
@@ -419,6 +421,7 @@ public:
   bool m_cancelWithLoadInProgress;
 
   QTimer m_lifeSupportTimer;
+  
 };
 
 #endif
index e0d8f4506c04119e52192244a6e1decde4001529..3368fc994bb1c2b1e2954c5ef6078eee23f7cba1 100644 (file)
@@ -287,8 +287,9 @@ public:
     
     void setMediaType(const QString &);
 
+    void setSelectionFromNone();
     void setDisplaysWithFocusAttributes(bool flag);
-    bool displaysWithFocusAttributes() const { return _displaysWithFocusAttributes; }
+    bool displaysWithFocusAttributes() const;
     
     // Convenience, to avoid repeating the code to dig down to get this.
     QChar backslashAsCurrencySymbol() const;
@@ -399,7 +400,6 @@ private:
 
     KWQWindowWidget *_windowWidget;
 
-    bool _displaysWithFocusAttributes;
     mutable bool _drawSelectionOnly;
     bool _haveUndoRedoOperations;
     
index e050213d9021962ab53b365d08b1440da83203b7..9f440ce733305cbff682b763ca7340672102d6ce 100644 (file)
@@ -191,7 +191,6 @@ KWQKHTMLPart::KWQKHTMLPart()
     , _formValuesAboutToBeSubmitted(nil)
     , _formAboutToBeSubmitted(nil)
     , _windowWidget(NULL)
-    , _displaysWithFocusAttributes(false)
     , _drawSelectionOnly(false)
     , _bindingRoot(0)
     , _windowScriptObject(0)
@@ -3420,25 +3419,13 @@ void KWQKHTMLPart::setMediaType(const QString &type)
     }
 }
 
-void KWQKHTMLPart::setDisplaysWithFocusAttributes(bool flag)
+void KWQKHTMLPart::setSelectionFromNone()
 {
-    if (_displaysWithFocusAttributes == flag)
-        return;
-    _displaysWithFocusAttributes = flag;
-        
-    // This method does the job of updating the view based on whether the view is "active".
-    // This involves three kinds of drawing updates:
-
-    // 1. The background color used to draw behind selected content (active | inactive color)
-    if (d->m_view)
-        d->m_view->updateContents(QRect(visibleSelectionRect()));
-
-    // 2. Caret blinking (blinks | does not blink)
-    //    Also, put the caret someplace if the selection is empty and the part is editable.
+    //    Put the caret someplace if the selection is empty and the part is editable.
     //    This has the effect of flashing the caret in a contentEditable view automatically 
     //    without requiring the programmer to set a selection explicitly.
     DocumentImpl *doc = xmlDocImpl();
-    if (doc && flag && selection().state() == Selection::NONE && isContentEditable()) {
+    if (doc && selection().state() == Selection::NONE && isContentEditable()) {
         NodeImpl *node = doc->documentElement();
         while (node) {
             // Look for a block flow, but skip over the HTML element, since we really
@@ -3450,9 +3437,28 @@ void KWQKHTMLPart::setDisplaysWithFocusAttributes(bool flag)
         if (node)
             setSelection(Position(node, 0));
     }
-    setCaretVisible(flag);
+}
 
+void KWQKHTMLPart::setDisplaysWithFocusAttributes(bool flag)
+{
+    if (d->m_isFocused == flag)
+        return;
+    d->m_isFocused = flag;
+        
+    // This method does the job of updating the view based on whether the view is "active".
+    // This involves three kinds of drawing updates:
+
+    // 1. The background color used to draw behind selected content (active | inactive color)
+    if (d->m_view)
+        d->m_view->updateContents(QRect(visibleSelectionRect()));
+
+    // 2. Caret blinking (blinks | does not blink)
+    if (flag)
+        setSelectionFromNone();
+    setCaretVisible(d->m_isFocused);
+    
     // 3. The drawing of a focus ring around links in web pages.
+    DocumentImpl *doc = xmlDocImpl();
     if (doc) {
         NodeImpl *node = doc->focusNode();
         if (node && node->renderer())
@@ -3460,6 +3466,11 @@ void KWQKHTMLPart::setDisplaysWithFocusAttributes(bool flag)
     }
 }
 
+bool KWQKHTMLPart::displaysWithFocusAttributes() const
+{
+    return d->m_isFocused;
+}
+
 QChar KWQKHTMLPart::backslashAsCurrencySymbol() const
 {
     DocumentImpl *doc = xmlDocImpl();
index 5d72be2a343c97e71699b5ccc1e07d7a6a1c3b89..09bb4dc33b0830e8c7128ca1c2dafe965aa3ff1d 100644 (file)
@@ -243,6 +243,7 @@ typedef enum {
 - (NSAttributedString *)selectedAttributedString;
 - (NSString *)selectedString;
 
+- (void)setSelectionFromNone;
 - (void)setDisplaysWithFocusAttributes:(BOOL)flag;
 
 - (NSString *)stringForRange:(DOMRange *)range;
index ee4dbdfc693b5c418328169992d9b656fef38ec6..c7262010d3b4a2aaf0c20a3eb2a48337da1aae30 100644 (file)
@@ -1259,6 +1259,11 @@ static HTMLFormElementImpl *formElementFromDOMElement(DOMElement *element)
     return externalRepresentation(_part->renderer()).getNSString();
 }
 
+- (void)setSelectionFromNone
+{
+    _part->setSelectionFromNone();
+}
+
 - (void)setDisplaysWithFocusAttributes:(BOOL)flag
 {
     _part->setDisplaysWithFocusAttributes(flag);
index bbeb966e0681f72ce92432c004abf9bcc877e212..acdd1c4c2d2fab94b60703568440560a0cbc339e 100644 (file)
@@ -1,3 +1,12 @@
+2004-09-20  Chris Blumenberg  <cblu@apple.com>
+
+       Fixed: <rdar://problem/3781290> REGRESSION (Mail): Crash in ReplaceSelectionCommandImpl attaching file to new message
+
+        Reviewed by kocienda.
+
+        * WebView.subproj/WebView.m:
+        (-[WebView setEditable:]): call updateSelectionFromEmpty on the bridge if there is no selection
+
 2004-09-20  Chris Blumenberg  <cblu@apple.com>
 
        Changes to implement renamed bridge methods.
index c7cb1c729eee7a08e475ed5a094fef6e85d30c31..e5648f0b2e03d4424e3ae4b28d8a0e2f3ba12951 100644 (file)
@@ -2437,7 +2437,13 @@ static WebFrame *incrementFrame(WebFrame *curr, BOOL forward, BOOL wrapFlag)
 
 - (void)setEditable:(BOOL)flag
 {
-    _private->editable = flag;
+    if (_private->editable != flag) {
+        _private->editable = flag;
+        if (flag && [self selectedDOMRange] == nil) {
+            // If the WebView is made editable and the selection is empty, set it to something.
+            [[[self mainFrame] _bridge] setSelectionFromNone];
+        }
+    }
 }
 
 - (BOOL)isEditable