From db90cd7983c0856cc97fb7ac3a6c768ebe44a222 Mon Sep 17 00:00:00 2001 From: "smichaud%pobox.com" Date: Thu, 17 Jan 2008 16:58:29 +0000 Subject: [PATCH] Fix a number of focus bugs on OS X. b=403232 r=joshmoz sr=roc git-svn-id: svn://10.0.0.236/trunk@243379 18797224-902f-48f8-a5cc-f745e15eee43 --- mozilla/widget/src/cocoa/nsChildView.h | 5 ++ mozilla/widget/src/cocoa/nsChildView.mm | 92 ++++++++++++++++--------- 2 files changed, 65 insertions(+), 32 deletions(-) diff --git a/mozilla/widget/src/cocoa/nsChildView.h b/mozilla/widget/src/cocoa/nsChildView.h index d4c7346c981..355ff13198c 100644 --- a/mozilla/widget/src/cocoa/nsChildView.h +++ b/mozilla/widget/src/cocoa/nsChildView.h @@ -131,7 +131,10 @@ union nsPluginPort; // Stop NSView hierarchy being changed during [ChildView drawRect:] - (void)delayedTearDown; + - (void)setTransparent:(BOOL)transparent; + +- (void)sendFocusEvent:(PRUint32)eventType; @end @@ -354,6 +357,8 @@ protected: PRPackedBool mPluginDrawing; PRPackedBool mPluginIsCG; // true if this is a CoreGraphics plugin + PRPackedBool mInSetFocus; + nsPluginPort mPluginPort; }; diff --git a/mozilla/widget/src/cocoa/nsChildView.mm b/mozilla/widget/src/cocoa/nsChildView.mm index 8ee437971e6..8d12cf40088 100644 --- a/mozilla/widget/src/cocoa/nsChildView.mm +++ b/mozilla/widget/src/cocoa/nsChildView.mm @@ -168,6 +168,17 @@ nsIWidget * gRollupWidget = nsnull; @end +// Used to retain an NSView for the remainder of a method's execution. +class nsAutoRetainView { +public: + nsAutoRetainView(NSView *aView) : mView([aView retain]) {} + ~nsAutoRetainView() { [mView release]; } + +private: + NSView *mView; // [STRONG] +}; + + #pragma mark - @@ -341,6 +352,7 @@ nsChildView::nsChildView() : nsBaseWidget() , mIsPluginView(PR_FALSE) , mPluginDrawing(PR_FALSE) , mPluginIsCG(PR_FALSE) +, mInSetFocus(PR_FALSE) { #ifdef PR_LOGGING if (!sCocoaLog) @@ -739,9 +751,55 @@ NS_IMETHODIMP nsChildView::IsEnabled(PRBool *aState) NS_IMETHODIMP nsChildView::SetFocus(PRBool aRaise) { + // Don't do anything if SetFocus() has been called reentrantly on the same + // object. Sometimes calls to nsChildView::DispatchEvent() can get + // temporarily stuck, causing calls to [ChildView sendFocusEvent:] and + // SetFocus() to be reentered. These reentrant calls are probably the + // result of one or more bugs, and doing things on a reentrant call can + // cause problems: For example if mView is already the first responder and + // we send it an NS_GOTFOCUS event (see below), this causes the Mochitests + // to get stuck in the toolkit/content/tests/widgets/test_popup_button.xul + // test. + if (mInSetFocus) + return NS_OK; + mInSetFocus = PR_TRUE; NSWindow* window = [mView window]; - if (window) - [window makeFirstResponder:mView]; + if (window) { + nsAutoRetainView kungFuDeathGrip(mView); + // 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 events (sent 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 + // events 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). + NSResponder* firstResponder = [window firstResponder]; + if ([mView isEqual:firstResponder]) { + // Sometimes SetFocus() is called on an nsChildView object that's + // already focused. In principle this shouldn't happen, and in any + // case we shouldn't have to dispatch any events. But if we don't, we + // sometimes get text-input cursors blinking in more than one text + // field, or still blinking when the browser is no longer active. For + // reasons that aren't at all clear, this problem can be avoided by + // always sending an NS_GOTFOCUS message here. + if ([mView isKindOfClass:[ChildView class]]) + [(ChildView *)mView sendFocusEvent:NS_GOTFOCUS]; + } else { + // Retain and release firstResponder around the call to + // makeFirstResponder. + [firstResponder retain]; + if ([window makeFirstResponder:mView]) { + if ([firstResponder isKindOfClass:[ChildView class]]) + [(ChildView *)firstResponder sendFocusEvent:NS_LOSTFOCUS]; + if ([mView isKindOfClass:[ChildView class]]) + [(ChildView *)mView sendFocusEvent:NS_GOTFOCUS]; + } + [firstResponder release]; + } + } + mInSetFocus = PR_FALSE; return NS_OK; } @@ -1760,17 +1818,6 @@ nsChildView::GetDocumentAccessible(nsIAccessible** aAccessible) #pragma mark - -// Used to retain an NSView for the remainder of a method's execution. -class nsAutoRetainView { -public: - nsAutoRetainView(NSView *aView) : mView([aView retain]) {} - ~nsAutoRetainView() { [mView release]; } - -private: - NSView *mView; // [STRONG] -}; - - @implementation ChildView @@ -4091,32 +4138,13 @@ static BOOL keyUpAlreadySentKeyDown = NO; } -// This method is called when we are about to be focused. -- (BOOL)becomeFirstResponder -{ - if (!mGeckoChild) - return NO; - - nsAutoRetainView kungFuDeathGrip(self); - - [self sendFocusEvent:NS_GOTFOCUS]; - - return [super becomeFirstResponder]; -} - - // This method is called when are are about to lose focus. // We must always call through to our superclass, even when mGeckoChild is // nil -- otherwise the keyboard focus can end up in the wrong NSView. - (BOOL)resignFirstResponder { - nsAutoRetainView kungFuDeathGrip(self); - nsTSMManager::CommitIME(); - if (mGeckoChild) - [self sendFocusEvent:NS_LOSTFOCUS]; - return [super resignFirstResponder]; }