582790 - When embedding Mozilla on Windows, dialog keyboard navigation and keyboard shortcuts for the embedding application sporadically fail

Status: UNCONFIRMED

(Core :: Widget: Win32, defect)

Description

[Dwayne Mercredi]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/533.4 (KHTML, like Gecko) Chrome/5.0.375.99 Safari/533.4
Build Identifier: XulRunner SDK 1.9.2.8pre snapshot 20100728135525

This is the root cause of bug 508901, which gives a good repro case.

I've tracked down the root problem.

When embedding Mozilla, nsBaseAppShell/nsAppShell periodically bypass the embedding application's main message loop and pumps windows messages while it's processing dispatched events.

Sequence of events:

1. nsBaseAppShell::NativeEventCallback() is called in response to a posted Windows message via nsAppShell::ScheduleNativeEventCallback()

2. mEventloopNestingState is eEventloopNone because nsAppShell doesn't control the top level message loop - that's done by the embedding application.

3. nsBaseAppShell::NativeEventCallback() calls NS_ProcessPendingEvents(), which (eventually) calls back to nsBaseAppShell::OnProcessNextEvent(), which pumps Windows messages.

The main symptom is that keyboard messages are sporadically handled incorrectly because IsDialogMessage(), PreTranslateMessage(), TranslateMessage(), etc. on the application's message loop aren't called when NativeEventCallback() is active.  Tabbing between controls suddenly inserts a "tab" character, Ctrl+O no longer opens an Outlook window, or other similar bugs occur.

There is already code in nsBaseAppShell::NativeEventCallback() that avoids pumping Windows messages by setting mBlockNativeEvent = PR_TRUE; when it detects that there is a non-mozilla modal loop, for example from a plug-in's dialog box or popup.

The attached diff fixes the problem for embedding scenarios.

Reproducible: Always

Comment 1

[Dwayne Mercredi]

Attachment: patch to work around the problem against xulrunner SDK 1.9.2

Comment 2

[Neil Deakin (away until Sept 6)]

Did you want someone to review or provide feedback on this patch? I'd suggest [e-mail]

Comment 3

[Dwayne Mercredi]

Attachment: An updated patch to remove the extraneous if statement

Comment 4

[Benjamin Smedberg]

Comment on attachment 461341 [details] [diff] [review]
An updated patch to remove the extraneous if statement

I don't own or know this code.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think you should check mEventloopNestingState != eEventloopXPCOM. I think trying to break out of the XPCOM event loop would break non-embedders.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 461341 [details] [diff] [review]
An updated patch to remove the extraneous if statement

See comment