344006 - Keyboard unresponsive, focus issues, after clicking on Java applet and switching windows

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[Uri Bernstein]

Clicking a Java applet and then switching to another window causes the keyboard to be unresponsive on the new window (you can't type anywhere). Sometimes, even just switching to the other window is difficult (requires clicking the title bar).

Steps to reproduce:
1. Open Firefox.
2. Navigate to a page including a Java Applet (such as the one in the URL field of this bug).
3. Open a second window.
4. Switch back to the first window by clicking on it.
5. Click somewhere on the applet (this step is important!)
6. Click on the second window to switch to it. At this point, you might already be unable to focus the second window without clicking the title bar).
7. Try to type something into the address bar, search bar, or any field in the window, or try using keyboard shortcuts to activate commands. This will all fail.

This regressed on the trunk between 2006-06-16 and 2006-06-17 and on the 1.8 branch between 2006-06-19 and 2006-06-20, so this is almost certainly a regression from bug 340592.

This is likely the same issue reported in bug 343815, but that one does not have steps to reproduce, so I'm making it a dependency for now.

Comment 1

[Mark Mentovai]

Probably mine

Comment 2

[Mark Mentovai]

Activation is all screwed up with multiple windows even before bug 340592.  I haven't tried logging the activate events yet, but I'm seeing the non-applet window retaining active chrome even when it's not the active window.  I know that this was changed slightly between the "E" and "F" versions of JEP, but even in the "F" version, activation is wonky.

It's no surprise that 340592 would trigger this given problems activating windows.

Comment 3

[Mark Mentovai]

Open two windows, window 1 and window 2
Activate window 1 and load Java applet therein
Click on window 2, observe events: win1 deactivate
                                   win2 activate
                                   win2 activate (again)
Click on window 1, observe events: win1 activate
                                   win1 activate (again)
Click on window 2, observe events: win1 deactivate
                                   win2 activate
Click on window 1, observe event:  win1 activate

Subsequent clicks on either window produce the same results as the last two clicks listed above.  The clicks above are in the window's content areas.  Things change slightly when the clicks are on the title bars: the first two clicks above are the same, but last two clicks above double up on events:

Click on window 2 title bar, observe events: win1 deactivate
                                             win1 deactivate (again)
                                             win2 activate
                                             win2 activate (again)
Click on window 1 title bar, observe events: win1 activate
                                             win1 activate (again)

When using command-` to switch windows, things behave slightly better in that activation and deactivation events are reliably sent to the windows.  The window that contains the applet does see more than one event on activation change, though, and it'll also see additional activate events whenever the command key is pressed.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

> Open two windows, window 1 and window 2
> Activate window 1 and load Java applet therein
> Click on window 2, observe events: win1 deactivate
>                                    win2 activate
>                                    win2 activate (again)
> ...

I assume the "activate" and "deactivate" events you're talking about
aren't Carbon events.  Am I right?

Comment 5

[Mark Mentovai]

You are.  (More to come.)

Comment 6

[Mark Mentovai]

Attachment: Activate/deactivate fix, checked in on trunk and 1.8.1

This patch takes advantage of the fact that at least one activate event is reliably delivered.  If an activate event is received without having received a deactivate event for the previously-active window, the last window is deactivated.  If an activate event is received for a window that's already active, it's dropped.  This patch shouldn't be necessary, but it provides a degree of paranoia.

It doesn't fix the key problem, which shows that activation as we see it in widget isn't really at the root of the issue (but maybe the activation and key problems share similar causes?)

Comment 7

[Mark Mentovai]

Now, hear this.  Activation is part of the problem, in that the attached patch provides activation workarounds that are enough to at least get keyup and keydown events sent to the right windows.

Even with that, there's another major problem: once a Java applet is loaded in the window, keydown and keyup events to that window are lost.  We [now] expect to take these events through raw key CE handlers.  The CE handlers aren't seeing any keyup/keydown events at all - they're not just directed at the wrong windows, they're gone.  Only keypress events are coming through (via TSM AE handlers).  This is a major bug that should be fixed in its own right.

The other problem, and it's at the root of this bug in combination with the activation problem, is that all keypress events are going to the event handler for the window hosting the applet, not the window that should be active.  This is a problem since bug 340592: it's now very important to direct events to the correct window, because the event dispatch manager is no longer global.  (This means that we're getting the wrong TSM doc refcon in the kUnicodeNotFromInputMethod AE handler, something I thought would also be fixed by the activation patch, but apparently wasn't.)

Comment 8

[Mark Mentovai]

Sorry, re comment 4, they ARE Carbon Events.

Comment 9

[Mark Mentovai]

Yup, the TSM doc is definitely wrong - it refers to the TSM doc of the window that is hosting the applet.  We (widget) always set up the TSM document on activation to correspond to the active window.  Something's changing the active TSM doc after we do.

Comment 10

[Mark Mentovai]

Attachment: Stack trace showing the wrong TSM doc being set

Looks like JEP needs to be careful about the fact that each window has its own TSM doc.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

There are a number of different circumstances in which the JEP needs
to change the current TSM doc.  This is (basically) because 1) each
Java applet has its own TSM docs (often plural), 2) bad things happen
when the current TSM doc is set incorrectly (usually crashes), and 3)
without the JEP's intervention the current TSM doc is usually set
incorrectly.

The code which does this (it exists in a bunch of different places in
the JEP) assumes that a given browser page will have at most one TSM
doc.  And (in my past experience) each browser page has exactly one
TSM doc.  Are these assumptions still correct?

Comment 12

[Mark Mentovai]

Each browser window has its own TSM doc.  Different tabs within the same window share the same TSM doc, and navigating between pages within a single window also uses one TSM doc.

Comment 13

[Mark Mentovai]

Forked off bugs:

Bug 344150 "Window activation problems when using JEP" for comments 2-3
Bug 344153 "Key-down/up events stop after loading JEP in a window" for the
           middle part of comment 7

Comment 14

[Mark Mentovai]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

I think we need this, because it fixes a problem with an extra deactivate event for the hidden window when restoring a minimized window by clicking on a URL in another app, as mentioned in bug 338642.  It also works around bug 344150.

Comment 15

[Mark Mentovai]

*** Bug 346123 has been marked as a duplicate of this bug. ***

Comment 16

[Steven Michaud [:smichaud] (Retired)]

I've just released version 0.9.5+g of the Java Embedding Plugin:

http://javaplugin.sourceforge.net/

It fixes the problems reported in comment #0, and also (I think) bug
344153, but not bug 344150.

With regard to the TSM doc:  I actually only found a very few places
where the Java Embedding Plugin set the TSM doc incorrectly (or might
do so).  I added appropriate checks in those places.  But I think most
TSM doc problems were actually side effects of the activation problem
(item #3 in JEP 0.9.5+g's Changes.txt) and bug 344701 (now fixed).

Comment 17

[Josh Aas]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

comments as discussed on irc

Comment 18

[Mark Mentovai]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

add braces to avoid giving josh a headache, fix misspelled comment (not mine)

Comment 19

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

sr=pink

Comment 20

[Mark Mentovai]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

This patch has been checked in on the trunk, fixing issues reported here, bug 344150 and parts of bug 338642.

We need this patch on the 1.8.1 branch for the following reason: open a single browser window, minimize it to the dock, switch to another application, and have it open an external URL.  The minimized browser window will raise and load the URL, but text input will not function.

The rest of this bug depends on JEP 0.9.5+g, bug 346156.

Comment 21

[Mike Connor [:mconnor]]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

a=mconnor on behalf of drivers for checkin to 1.8 branch for 1.8.1

Comment 22

[Mark Mentovai]

Comment on attachment 228701 [details] [diff] [review]
Activate/deactivate fix, checked in on trunk and 1.8.1

This has been checked in on MOZILLA_1_8_BRANCH for 1.8.1b2.

Comment 23

[Mark Mentovai]

Marking FIXED and fixed1.8.1 because attachment 228701 [details] [diff] [review] has been checked in.  The rest of this bug depends on JEP 0.9.5+g landing, bug 346156.

Comment 24

[Mark Mentovai]

JEP 0.9.5+g checked in.