From 6d86e37dc1323f70ec58e98954fabff883c6b4c9 Mon Sep 17 00:00:00 2001 From: "smichaud@pobox.com" Date: Tue, 18 Dec 2007 16:35:14 -0800 Subject: [PATCH] Fix multiple focus bugs. b=403232 r=joshmoz sr=roc --- widget/src/cocoa/nsChildView.h | 6 +++ widget/src/cocoa/nsChildView.mm | 87 +++++++++++++++++++++++++++++++-- 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/widget/src/cocoa/nsChildView.h b/widget/src/cocoa/nsChildView.h index 12f9ab4c2ad..aa52f856f9d 100644 --- a/widget/src/cocoa/nsChildView.h +++ b/widget/src/cocoa/nsChildView.h @@ -91,6 +91,9 @@ union nsPluginPort; // Whether we're a plugin view. BOOL mIsPluginView; + // Whether we're in nsChildView::SetFocus() + int mInSetFocusLevel; + NSEvent* mCurKeyEvent; // only valid during a keyDown PRBool mKeyDownHandled; @@ -127,6 +130,9 @@ union nsPluginPort; // Stop NSView hierarchy being changed during [ChildView drawRect:] - (void)delayedTearDown; + +-(void)setInSetFocus:(BOOL)aInSetFocus; +-(BOOL)inSetFocus; @end diff --git a/widget/src/cocoa/nsChildView.mm b/widget/src/cocoa/nsChildView.mm index 119f20e24a5..2ccae73114b 100644 --- a/widget/src/cocoa/nsChildView.mm +++ b/widget/src/cocoa/nsChildView.mm @@ -718,11 +718,64 @@ NS_IMETHODIMP nsChildView::IsEnabled(PRBool *aState) } +class nsAutoSetInSetFocus { + public: + nsAutoSetInSetFocus(NSView *aView) + { + if ([aView isKindOfClass:[ChildView class]]) { + mChildView = (ChildView *) [aView retain]; + [mChildView setInSetFocus:PR_TRUE]; + } else { + mChildView = nil; + } + } + ~nsAutoSetInSetFocus() + { + if (mChildView) { + [mChildView setInSetFocus:PR_FALSE]; + [mChildView release]; + } + } + + private: + ChildView *mChildView; // [STRONG] +}; + + NS_IMETHODIMP nsChildView::SetFocus(PRBool aRaise) { NSWindow* window = [mView window]; - if (window) - [window makeFirstResponder:mView]; + if (window) { + // For reasons that aren't yet clear, focus changes within a window (as + // opposed to those between windows or between apps) should only trigger + // NS_LOSTFOCUS and NS_GOTFOCUS messages (to Gecko) in the context of a + // call to nsChildView::SetFocus() (or nsCocoaWindow::SetFocus(), which + // in any case re-routes to nsChildView::SetFocus()). If we send these + // messages on every intra-window focus change (on every call to + // [ChildView becomeFirstResponder:] or [ChildView resignFirstResponder:]), + // the result will be strange focus bugs (like bmo bugs 399471, 403232, + // 404433 and 408266). + nsAutoSetInSetFocus setInSetFocus(mView); + NSResponder* firstResponder = [window firstResponder]; + if ([mView isEqual:firstResponder]) { + // Sometimes SetFocus() is called on an nsChildView object that's + // already focused. If we simply call [NSWindow makeFirstResponder:], + // neither [ChildView becomeFirstResponder:] nor [ChildView + // resignFirstResponder:] will get called, and no NS_LOSTFOCUS or + // NS_GOTFOCUS messages will be sent. But in this case we sometimes + // get text-input cursors blinking in more than one text field. So we + // need to guarantee that the code in nsEventStateManager:: + // PreHandleEvent() which handles NS_LOSTFOCUS messages (and calls + // SetContentCaretVisible(... PR_FALSE)) gets invoked on every call to + // nsChildView::SetFocus(). + if ([mView isKindOfClass:[ChildView class]]) { + [(ChildView *)mView sendFocusEvent:NS_LOSTFOCUS]; + [(ChildView *)mView sendFocusEvent:NS_GOTFOCUS]; + } + } else { + [window makeFirstResponder:mView]; + } + } return NS_OK; } @@ -1763,6 +1816,7 @@ NSEvent* gLastDragEvent = nil; mWindow = nil; mGeckoChild = inChild; mIsPluginView = NO; + mInSetFocusLevel = 0; mCurKeyEvent = nil; mKeyDownHandled = PR_FALSE; @@ -2074,6 +2128,24 @@ NSEvent* gLastDragEvent = nil; } +-(void)setInSetFocus:(BOOL)aInSetFocus +{ + if (aInSetFocus) { + ++mInSetFocusLevel; + } else if (mInSetFocusLevel > 0) { + --mInSetFocusLevel; + } else { + NS_WARNING("ChildView setInFocus: inSetFocus already false"); + } +} + + +-(BOOL)inSetFocus +{ + return mInSetFocusLevel > 0; +} + + - (void)sendFocusEvent:(PRUint32)eventType { if (!mGeckoChild) @@ -4086,7 +4158,11 @@ static BOOL keyUpAlreadySentKeyDown = NO; if (!mGeckoChild) return NO; - [self sendFocusEvent:NS_GOTFOCUS]; + // Focus events should only be sent to Gecko here in the context of a call + // to nsChildView::SetFocus(). For more info see nsChildView::SetFocus() + // above. + if ([self inSetFocus]) + [self sendFocusEvent:NS_GOTFOCUS]; return [super becomeFirstResponder]; } @@ -4099,7 +4175,10 @@ static BOOL keyUpAlreadySentKeyDown = NO; { nsTSMManager::CommitIME(); - if (mGeckoChild) + // Focus events should only be sent to Gecko here in the context of a call + // to nsChildView::SetFocus(). For more info see nsChildView::SetFocus() + // above. + if (mGeckoChild && [self inSetFocus]) [self sendFocusEvent:NS_LOSTFOCUS]; return [super resignFirstResponder];