499805 - Focus broken in dialog boxes on Windows CE

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Since bug 178324 landed, focus is broken on dialog boxes on Windows CE; no input boxes can be typed into (simple example is just a js prompt(), but also firefox prefs dialog).  I've confirmed that the OS sending SETFOCUS/ACTIVATE messages to the correct window, Gecko just doesn't seem to recognize that those edit boxes have focus.  Unfortunately, Spy++ isn't working for me, so I can't trace messages that way.

Comment 2

[Neil Deakin]

If you enable the debugging defines at the top of nsFocusManager.cpp I can examine the output to see what is happening.

Is it just edit fields that don't get focused? Or all elements?

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

seems like all elements.  I hit tab repeatedly, but nothing got a focus right.  For what its worth, I can still click on the buttons.

Comment 4

[Doug Turner (:dougt)]

Window 51C61480 Hidden [Currently: 51C5FC00 51C5FC00] <<Hide Window: about:blank Focused Window: chrome://browser/content/browser.xul Active Window: chrome://browser/content/browser.xul>>
Window 51C61480 Shown [Currently: 51C5FC00 51C5FC00] <<Shown Window: chrome://global/content/commonDialog.xul Focused Window: chrome://browser/content/browser.xul>>
WM_ACTIVATE
WM_ACTIVATE
Window 51C5FC00 Lowered [Currently: 51C5FC00 51C5FC00] <<[51C5FC00] Lowered Window: chrome://browser/content/browser.xul [51C5FC00] Active Window: chrome://browser/content/browser.xul>>
ResetInputState
SetIMEEnabled: Disabled
**Element input has been blurred
ResetInputState
WM_SETFOCUS
Window 51C5FC00 Raised [Currently: 00000000 00000000] <<[51C5FC00] Raised Window: chrome://browser/content/browser.xul>>
<<MoveFocus Type: 1 Flags: 0>>
<<>> $[[(none)]]
Focus Navigation Start Content dialog
[Tabindex: 1 Ignore: 0]GetNextTabbable: dialog tabindex: 1
Next Tabbable dialog: with tabindex: -1 expected: 1
Next Tabbable popupgroup: with tabindex: -1 expected: 1
Next Tabbable vbox: with tabindex: -1 expected: 1
Next Tabbable popupset: with tabindex: -1 expected: 1
Next Tabbable hbox: with tabindex: -1 expected: 1
Next Tabbable spacer: with tabindex: -1 expected: 1
Next Tabbable grid: with tabindex: -1 expected: 1
Next Tabbable columns: with tabindex: -1 expected: 1
Next Tabbable column: with tabindex: -1 expected: 1
Next Tabbable column: with tabindex: -1 expected: 1
Next Tabbable rows: with tabindex: -1 expected: 1
Next Tabbable row: with tabindex: -1 expected: 1
Next Tabbable hbox: with tabindex: -1 expected: 1
Next Tabbable image: with tabindex: -1 expected: 1
Next Tabbable vbox: with tabindex: -1 expected: 1
Next Tabbable description: with tabindex: -1 expected: 1
Next Tabbable description: with tabindex: -1 expected: 1
Next Tabbable #text: with tabindex: -1 expected: 1
Next Tabbable row: with tabindex: -1 expected: 1
Next Tabbable label: with tabindex: -1 expected: 1
Next Tabbable textbox: with tabindex: -1 expected: 1
Next Tabbable hbox: with tabindex: -1 expected: 1
Next Tabbable input: with tabindex: 0 expected: 1
Next Tabbable div: with tabindex: -1 expected: 1
Next Tabbable div: with tabindex: -1 expected: 1
Next Tabbable br: with tabindex: -1 expected: 1
Next Tabbable hbox: with tabindex: -1 expected: 1
Next Tabbable button: with tabindex: 0 expected: 1
Next Tabbable hbox: with tabindex: -1 expected: 1
Next Tabbable image: with tabindex: -1 expected: 1
Next Tabbable label: with tabindex: -1 expected: 1
Next Tabbable button: with tabindex: 0 expected: 1
Next Tabbable hbox: with tabindex: -1 expected: 1
Next Tabbable image: with tabindex: -1 expected: 1
Next Tabbable label: with tabindex: -1 expected: 1
Next Tabbable dialog: with tabindex: -1 expected: 0
Next Tabbable popupgroup: with tabindex: -1 expected: 0
Next Tabbable vbox: with tabindex: -1 expected: 0
Next Tabbable popupset: with tabindex: -1 expected: 0
Next Tabbable hbox: with tabindex: -1 expected: 0
Next Tabbable spacer: with tabindex: -1 expected: 0
Next Tabbable grid: with tabindex: -1 expected: 0
Next Tabbable columns: with tabindex: -1 expected: 0
Next Tabbable column: with tabindex: -1 expected: 0
Next Tabbable column: with tabindex: -1 expected: 0
Next Tabbable rows: with tabindex: -1 expected: 0
Next Tabbable row: with tabindex: -1 expected: 0
Next Tabbable hbox: with tabindex: -1 expected: 0
Next Tabbable image: with tabindex: -1 expected: 0
Next Tabbable vbox: with tabindex: -1 expected: 0
Next Tabbable description: with tabindex: -1 expected: 0
Next Tabbable description: with tabindex: -1 expected: 0
Next Tabbable #text: with tabindex: -1 expected: 0
Next Tabbable row: with tabindex: -1 expected: 0
Next Tabbable label: with tabindex: -1 expected: 0
Next Tabbable textbox: with tabindex: -1 expected: 0
Next Tabbable hbox: with tabindex: -1 expected: 0
Next Tabbable input: with tabindex: 0 expected: 0
Next Content: input
-> Element to be focused: input

!!!! about right here, i start clicking on the text field a bunch:


Shift Focus: input Flags: 0 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<MoveFocus end>>
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0
<<SetFocus>>
Shift Focus: input Flags: 4098 Current Window: 00000000 New Window: 51C61480 Current Element: 00000000 In Active Window: 0 In Focused Window: 0

Comment 5

[Neil Deakin]

(In reply to comment #4)
> Window 51C61480 Hidden [Currently: 51C5FC00 51C5FC00] <<Hide Window:
> about:blank Focused Window: chrome://browser/content/browser.xul Active Window:
> chrome://browser/content/browser.xul>>
> Window 51C61480 Shown [Currently: 51C5FC00 51C5FC00] <<Shown Window:
> chrome://global/content/commonDialog.xul Focused Window:
> chrome://browser/content/browser.xul>>
> WM_ACTIVATE
> WM_ACTIVATE

I'm assuming that the first WM_ACTIVATE here is an attempt to activate the dialog? And that the second is a deactivation of the browser window via minimizing it?

> Window 51C5FC00 Lowered [Currently: 51C5FC00 51C5FC00] <<[51C5FC00] Lowered
> Window: chrome://browser/content/browser.xul [51C5FC00] Active Window:
> chrome://browser/content/browser.xul>>
> ResetInputState
> SetIMEEnabled: Disabled
> **Element input has been blurred
> ResetInputState
> WM_SETFOCUS
> Window 51C5FC00 Raised [Currently: 00000000 00000000] <<[51C5FC00] Raised
> Window: chrome://browser/content/browser.xul>>

Looks as the focus system is being told that browser.xul is being lowered and then raised again. After the call to nsFocusManager::WindowRaised, it doesn't think that a window is focused. Although it appears to realize it's active as pressing Tab recognizes itself as occuring in the dialog.

I assume that assertions are enabled in this log? If so, the only means of failing would be if nsFocusManager::Focus returned early.

I wonder if the window visibility check is failing as happened with other offscreen bugs? Is there some special offscreen variation happening on CE?

Comment 6

[Neil Deakin]

But then again, maybe the problem is that the dialog should be raised after the WM_SETFOCUS event, not browser.xul.

Comment 7

[Doug Turner (:dougt)]

I do not see nsWindow::IsVisible() being called from any of the focus code when opening a the js prompt().  Is there another predicate?

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Neil, this is a blocker for a few things, including some partner prerelease builds -- what can we do to get you the information that you need to fix it?

Comment 9

[Neil Deakin]

> > WM_ACTIVATE
> > WM_ACTIVATE
> 
> I'm assuming that the first WM_ACTIVATE here is an attempt to activate the
> dialog? And that the second is a deactivation of the browser window via
> minimizing it?

Could you tell me which window/widget/frame the wm_activate and wm_setfocus events are firing on? Is it an activate or deactivate?

(In reply to comment #7)
> I do not see nsWindow::IsVisible() being called from any of the focus code when
> opening a the js prompt().  Is there another predicate?

You probably want to check to see what nsDocShell::GetVisibility is doing.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I traced this earlier, but figured brad posted the better data -- but the activate/setfocus events are being called on the "correct" windows, and in the correct order, as far as nsWindow expects.  I don't have the log, though.

Comment 11

[Neil Deakin]

What I see from the log is that:

1. The browser window is minimized and sent an WM_ACTIVATE
2. The dialog is activated and sent an WM_ACTIVATE
3. The WM_SETFOCUS is being sent to the right dialog, but the focus manager is receiving it as if it was the other browser window that was being focused. (As the same window is implied by the 'Window 51C5FC00 Lowered' and 'Window 51C5FC00 Raised' lines)

I believe the WM_SETFOCUS is sent to right window also because the subsequent tab key presses get through to the dialog. Also, the raise window is failing for some other reason, possibly related to the window's enabled state, that is, nsWindow::IsEnabled may be returning false because the window is minimized.

That might imply that the call to GetTopLevelHWND from within DispatchFocusToTopLevelWindow isn't returning the right widget (or possibly the call to GetNSWindowPtr), causing the system focus message to be redirected to the wrong place. Someone should focus debugging efforts here to start with.

As an aside, I noticed that there's a line in nsWindow::GetEnabled which looks rather silly:

  *aState = !mWnd || (::IsWindowEnabled(mWnd) && ::IsWindowEnabled(mWnd));

Comment 12

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Looks like DispatchFocusToTopLevelWindow is getting the wrong window from GetTopLevelHWND:

700224C0/70022A40 are the old window
7002D520 is new window

11832320 PID:7970376 TID:7a806d6 WM_ACTIVATE: hwnd: 700224C0 fActive: 0 hiword: 0
11832324 PID:7970376 TID:7a806d6 WM_ACTIVATE: hwnd: 7002D520 fActive: 1 hiword: 0

11832327 PID:7970376 TID:7a806d6 WM_KILLFOCUS: hwnd: 70022A40 JustGotDeactivate: 1
11832329 PID:7970376 TID:7a806d6 DispatchFocusToTopLevelWindow: mWnd: 70022A40 toplevel: 700224C0
11832332 PID:7970376 TID:7a806d6 Window 004AD500 Lowered [Currently: 004AD500 004AD500] <<
11832334 PID:7970376 TID:7a806d6 [004AD500] Lowered Window: file:///Hard%20Disk/tmp/ftest.html
11832337 PID:7970376 TID:7a806d6  [004AD500] Active Window: file:///Hard%20Disk/tmp/ftest.html
11832339 PID:7970376 TID:7a806d6 >>
11832342 PID:7970376 TID:7a806d6 **Element button has been blurred

11832346 PID:7970376 TID:7a806d6 WM_SETFOCUS: hwnd: 7002D520 JustGotActivate: 1
11832349 PID:7970376 TID:7a806d6 DispatchFocusToTopLevelWindow: mWnd: 7002D520 toplevel: 700224C0
11832351 PID:7970376 TID:7a806d6 Window 004AD500 Raised [Currently: 00000000 00000000] <<
11832353 PID:7970376 TID:7a806d6 [004AD500] Raised Window: file:///Hard%20Disk/tmp/ftest.html
11832356 PID:7970376 TID:7a806d6 >>
11832414 PID:7970376 TID:7a806d6 <<MoveFocus Type: 1 Flags: 0>>
<<
11832417 PID:7970376 TID:7a806d6 >> $[[(none)]]
11832435 PID:7970376 TID:7a806d6 Shift Focus: input
11832438 PID:7970376 TID:7a806d6  Flags: 0 Current Window: 00000000 New Window: 004ADF80 Current Element: 00000000
11832440 PID:7970376 TID:7a806d6  In Active Window: 0 In Focused Window: 0
11832444 PID:7970376 TID:7a806d6 <<MoveFocus end>>


this is the result of opening a prompt() from a simple html window.

So, 700224C0 is showing up as the top level parent of everything, which isn't correct..

Comment 13

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix

This seems to fix it; the problem is that GetParent will go back up to owner windows, not just parents, so we'll blow past the toplevel window if the window has an owner. We explicitly check for that and stop searching.

This passed win32 unit tests on the try server; I have no idea how this wasn't a problem on the desktop.

Comment 14

[Neil Deakin]

Comment on attachment 387263 [details] [diff] [review]
fix

>-    curWnd = ::GetParent(curWnd); // Parent or owner (if has no parent)
>+    upWnd = ::GetParent(curWnd); // Parent or owner (if has no parent)
>+
>+    // Then check if it's the owner, since we just want the parent
>+    if (upWnd && ::GetWindow(curWnd, GW_OWNER) == upWnd) {
>+      topWnd = curWnd;

topWnd should already be set to curWnd.

Comment 15

[Neil Deakin]

Actually, this breaks some things with popups, so we should make this WinCE only for now (or fix this actual problem)

1. open the add bookmarks panel by clicking the star
2. open the menulist inside it
3. focus the textbox and then try to type

Comment 16

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

We've got something pretty broken going on.  The popup menu from the star works fine without my patch, but with it it breaks.  I modified my patch to traverse to the owner window if we the current window is a popup (WS_POPUP), and that fixed the add bookmark popup, but broke prompt() again.  Trying to figure it all out...

Comment 17

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix v2

This fixes both cases... I don't know why, but it seems to work fine.  Focus gets given to popups (bookmarks thing) and dialogs (prompt, prefs).

Comment 18

[Neil Deakin]

Comment on attachment 387347 [details] [diff] [review]
fix v2

>   VERIFY_WINDOW_STYLE(style);
>   return style;
>-}
>\ No newline at end of file
>+}

No reason to change the endline, but otherwise is OK.

Comment 19

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

http://hg.mozilla.org/mozilla-central/rev/71019386aa86

Comment 20

[Tony Chung [:tchung]]

Verified fix on NV Tegra build: Mozilla/5.0 (Windows; U; WindowsCE 6.0; en-US; rv:1.9.2a1pre) Gecko/20090727 Firefox/3.6a1pre.