516274 - OS/2 plugin focus issues

Status: RESOLVED FIXED

(Core Graveyard :: Widget: OS/2, defect)

Description

[Rich Walsh]

As described in Bug 500379, if a plugin window gets the focus it won't let go when you click on the page.  You have to click on a text entry window, then click on the page before you can use the mouse wheel or arrow keys.

In addition, if a text entry window has the focus, the cursor & highlighting aren't removed when you click on the plugin.  Clicking on the text window again fails to restore focus (although the cursor is there & blinking).  As above, you have to click elsewhere, then return to it.

It appears that the browser isn't aware that a given element has lost focus, so it sees no need to restore focus to it when you click on it again.

Comment 1

[Rich Walsh]

Attachment: an omnibus focus fix

This fixes all focus issues I can reproduce, both general & plugin-related, and possibly, one I can't.  Here's an overview of what's changed:

General - We were having activation & focus problems because the NS_ACTIVATE/DEACTIVATE events were being misdirected to child widgets rather than the top-level widget.  A new virtual function, ActivateTopLevelWidget(), ensures that won't happen again.

The global flags used to signal window activation/deactivation have been eliminated.  Activation now sets a flag that's a member of nsFrameWindow to ensure that only that widget can act upon it.  This may resolve the problem reported in the newsgroups where clicking on a minimized window or selecting it from the window list activates the wrong window.  For deactivation, NS_DEACTIVATE is fired immediately since it doesn't affect the focus.

Plugins - Plugin (and window) activation events depend on nsWindow receiving a WM_FOCUSCHANGED msg.  However, an OS/2 plugin's visible windows aren't widgets, so nsWindow never gets the message when you click on them.  This problem is resolved by having the plugin widget synthesize this msg whenever one of its children gets the focus.  The msg makes it look like the widget got the focus, triggering the appropriate sequence of events.

The only remaining issue involves the GBM plugin:  it frequently steals the focus.  Heiko Nitzsche will release a new version within a month to fix this.

Comment 2

[Rich Walsh]

Whatever changes have happened in the last couple of weeks has completely broken this patch's plugin focus handling.  I'll either update it or file a new bug when I find the cause.

Comment 3

[Rich Walsh]

(In reply to comment #2)
> Whatever changes have happened in the last couple of weeks has completely
> broken this patch's plugin focus handling.

False alarm...  I did a fresh pull, built a debug version, and now it all works.

Comment 4

[Peter Weilbacher]

Comment on attachment 404769 [details] [diff] [review]
an omnibus focus fix

I'm happy to see some of the old kudges go, like gJustGot(De)Activate. :-)

Comment 5

[Peter Weilbacher]

http://hg.mozilla.org/mozilla-central/rev/dbebe1771118

Comment 6

[Rich Walsh]

I consider this to be a major fix that should be pushed to the branches as well.  I'll look at them to see if any changes are needed;  if so, I'll open a new bug for that branch.

Comment 7

[Rich Walsh]

(In reply to comment #6)
> I consider this to be a major fix that should be pushed to the branches as
> well.  I'll look at them to see if any changes are needed;

This patch can be applied to 1.9.2 without any changes - FF 3.6 et al. will definitely benefit from it.

OTOH, 1.9.1 still uses the old focus system, so a rewrite would be necessary. I don't think that's a good idea:  the risk/reward ratio is too high.

Comment 8

[Peter Weilbacher]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/c16794559aed

(Oops, maybe I should have asked for approval, as this patch contained one tiny change to dom/base/nsFocusManager.cpp. But I don't think it's worth backing it out for that, as it passed on trunk.)

Comment 9

[Peter Weilbacher]

This was apparently too late for beta1, at least the tag FIREFOX_3_6b1_RELEASE does not contain the change.