306245 - Page loading in background tab steals keyboard focus from textarea in foreground tab (when bg tab jumps to anchor)

Status: RESOLVED WORKSFORME

(Camino Graveyard :: Accessibility, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I'm not sure the summary is adequate/complete.  This has been happening for the
past few days, but I'm not sure exactly when since I've been pretty sporadic in
grabbing builds this past week :-(  Don't know if happens on the trunk or not.

I'm not sure it's every page loading in the bg, but I can repro it consistently
when posting replies on phpBB-driven fora (like MozillaZine).

STR:
1. Open several forum topics in tabs (make sure at least one is several
screen-heights long so you will have plenty of time to space bar-page down)

2. Reply to one topic; after pressing the submit button, switch tabs to the
"long" tab.

3. In your "long" tab, press the spacebar periodically to page down.  Note that
at some point as the reply-tab loads the "message has been submitted" page and
then reloads the full topic in the background, the current tab loses keyboard
focus and spacebar no longer pages down.

Comment 1

[Mike Pinkerton (not reading bugmail)]

i've seen this as well, it's annoying as heck. definitely a regression from 08.

Comment 2

[Mike Pinkerton (not reading bugmail)]

does it also happen in FF trunk?

Comment 3

[Simon Fraser [no longer active]]

I still see windows popup to the front occastionally too.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #2)
> does it also happen in FF trunk?

No.  Nor on Fx branch.  So it's Camino-only.  I wonder if it was one of the
checkins on the 25th?

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

This seems to be WFM in the 2005090804 (v0.9a2+) build in some very limited
usage.  Can anyone else confirm?

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I just saw this again, but not space bar up/down a page anymore, instead when I
was typing in a text box, the focus was stolen by the bg-loading tab.

Comment 7

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I did some more tests and am positive that comment 0 is no longer present;
comment 6, however, is.

Comment 8

[Chris Lawson (gone)]

I'm not sure if it helps at all, but I'm seeing the tab chain focus get moved to
the Bookmarks bar, of all places, when trying to tab back to text fields
de-focused by this bug.

cl

Comment 9

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #8)
> I'm not sure if it helps at all, but I'm seeing the tab chain focus get moved to
> the Bookmarks bar, of all places, when trying to tab back to text fields
> de-focused by this bug.

Probably bug 198153.

I'll see if I can't get a regression window for this tonight.  It began when I
was away and not getting builds regularly and since then I haven't had time to
sit down and grab bunches of builds to check :-(

Comment 10

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

OK, so we caught this before it was a year old.  That's nice :-)

This (comment 6) regressed between 
Camino 2004102808 (v0.8+) - works
Camino 2004102908 (v0.8+) - broken

Regression window: http://tinyurl.com/72fz4

Closest sounding checkin looks like aaronl with bug 258514 (when the final page
is (re)loaded, it's jumping to an anchor linking to the individual new post.)

Updated STR for this regression (comment 6):

1. Open a phpBB2-based forum in one tab (e.g., MozillaZine) and something else
with a textarea in a second tab (e.g., this bug).
2. Make a post/reply in the forum.
3. After hitting Submit, switch to the second tab and start typing in the textarea.
4. Continue typing as the "intermediate" page loads and then as the forum thread
with your new posts loads in the first, bg tab.
5. Observe that when the page in the first tab finally completes loading, you
suddenly stop seeing letters entered by keypresses.

(Oddly, you can, however, press tab after step 5 and end up in the next
focusable element, which at that time in Camino was the "mark as duplicate of
bug #" field, so keyboard focus is not completely lost.)

This is a Camino-only regression (at least, comment 6 does not appear in today's
Fx 1.8-branch build); not sure if it needs a blocking1.8??? nomination.

(Note that at the time of the regression, "space bar = page down" was broken
generally--it seems to regress frequently on the trunk.  But this bug is about
comment 6, as comment 0 cleared up since I filed the bug originally and has not
reappeared.)

Comment 11

[Samuel Sidler (old account; do not CC)]

Requesting blocking1.8rc1.

As far as we can tell, this broke on a core checkin as mentioned in comment 10.
Could someone look at this and see if it can be fixed for 1.8? The regression
effects Camino only.

Comment 12

[Scott MacGregor]

Mike, how's this bug going? Not much activity in the last few days. Do you need
any  help? Thanks. 

Comment 13

[Asa Dotzler [:asa]]

Johnny, we need your help. Can you look into this or help us find someone who can?

Comment 14

[Simon Fraser [no longer active]]

Attachment: Testcase

Test page that focusses the textarea after 4 seconds. Load this in another tab
and try to keeep typing in the bug.

Comment 15

[Simon Fraser [no longer active]]

Well, this all seems to be happening in nsEventStateManager::SendFocusBlur(). I
don't seen any provision in that code to prevent the setting of focus in a
background tab from stealing focus from the frontmost tab (and, yes, it reaches
across web browsers via a bunch of "gLastFocused" globals).

Aaron, how is this supposed to work?

Comment 16

[Johnny Stenback (:jst)]

This is the Camino version of bug 124750. There's *no* way we're fixing that the
right way for 1.8, we worked around it in that bug, and that fixed the problems
for Firefox/SeaMonkey. Camino needs to behave the same way to get the same
workaround. The key is that a hidden tab's nsIBaseWindow::GetVisible() needs to
return false, if it does, we won't focus things.

Mike, I'm gonna give this to you, and if you're not the right Camino developer
to deal with this then please reassign to the right person.

Comment 17

[Simon Fraser [no longer active]]

I don't see nsIBaseWidget::IsVisible() get called at all when the background tab
does a focus(). Even if it was, we should be returning the correct value.

Comment 18

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

In the STR in comment 10, there's no JS focusing a textbox or anything; it's
merely loading a page / jumping to an anchor (the site I used for the test
doesn't have the "Quick Reply" box that mozillazine does, but mZ doesn't appear
to put focus in the "Quick Reply" box anyway).  It's also a regression on
2004-10-29-08.

Bug 124750 was opened in 2002, though; it seems to correspond to the testcase in
comment 14, but not to the regression in comment 6 / comment 10--at least to me. 

Comment 19

[Simon Fraser [no longer active]]

(In reply to comment #17)
> I don't see nsIBaseWidget::IsVisible() get called at all when the background tab
> does a focus(). Even if it was, we should be returning the correct value.

Never mind; I was thinking of nsIWidget::IsVisible(). Testing GetVisibility() now.

Comment 20

[Simon Fraser [no longer active]]

Camino's embedding nsIBaseWindow::GetVisibility() is not called. The code in
nsGenericElement::ShouldFocus() that I assume should be calling this ends up
getting an nsIBaseWindow implemented by nsDocShell instead. DocShell's
implementation just looks at view visibility.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Simon, how is the nsIBaseWindow that actually has the right visibility related
to the nsIWebBrowser in question?  nsIWebBrowser::GetVisibility just calls
through to the docshell, so I'm assuming there's yet another GetVisibility
method involved?

What we probably need to do in the end (and this is where GetInterface is going
to make life suck) is to draw a distinction between "docshell" and "window
containing rendering area"; the latter would be the docshell or the nsWebBrowser
or something else, depending on the setup...

Comment 22

[Johnny Stenback (:jst)]

Note also that bug 310825 fixes a problem very similar to this when
window.focus() is called (unlike someelement.focus()). That fix is to do more
IsVisilble() checking etc. But I thought this bug was specifically about
focusing elements in a background tab, if not, this is problably a dupe of bug
310825.

Comment 23

[Boris Zbarsky [:bzbarsky]]

That patch has the same issue as the patch for bug 124750, Johnny -- it only
works in a non-embedding environment.  In embedding, docshells always claim to
be visible.

Comment 24

[Johnny Stenback (:jst)]

Attachment: Give embedding apps a chance to say whether a base window is visible or not.

Per suggestions from bz this makes the docshell ask the docshell owner
(embedding code) whether the owner's base window is visible or not, and that
way an embedding app's docshell owner code can say whether a tab is visible or
not. Not tested in an embedded app, but works in Firefox. Camino peopl, please
test, if this works we could try to get this in for 1.8, but that means we need
to hurry up...

Comment 25

[Simon Fraser [no longer active]]

Comment on attachment 200023 [details] [diff] [review]
Give embedding apps a chance to say whether a base window is visible or not.

This patch works, once I fix up CHBrowserListener::GetVisibility() in Camino.

Comment 26

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 200023 [details] [diff] [review]
Give embedding apps a chance to say whether a base window is visible or not.

sr=bzbarsky, though it feels like patch-cest.  ;)

Comment 27

[Simon Fraser [no longer active]]

Attachment: Camino part of patch

Comment 28

[Mark Mentovai]

Comment on attachment 200036 [details] [diff] [review]
Camino part of patch

Camino portion looks good.

Comment 29

[Boris Zbarsky [:bzbarsky]]

I checked in the core change to trunk.  I assume this is something we want on
the 1.8 branch?

Comment 30

[Johnny Stenback (:jst)]

Comment on attachment 200023 [details] [diff] [review]
Give embedding apps a chance to say whether a base window is visible or not.

Yeah, I think we do want this on the branch...

Comment 31

[Asa Dotzler [:asa]]

can we get this resolved and verified on the trunk today so we can properly
evaluate for the branch? thanks.

Comment 32

[Simon Fraser [no longer active]]

I checked in the Camino part of the changes (attachment 200036 [details] [diff] [review]) into the trunk
and branch.

Comment 33

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk, then.  Can someone who has camino verify on trunk?

Comment 34

[Mark Mentovai]

I confirm that this is now fixed in my current Camino trunk build.

Comment 35

[Johnny Stenback (:jst)]

Fix landed on the 1.8 branch.

Comment 36

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I just downloaded the Camino 2005102008 (v1.0+) trunk nightly (which should be
the only one which has both parts of this fix), and this bug (the regression in
comment 6 / STR in comment 10) *is not* fixed.  

The bg tab jumping to the anchor while (re)loading still steals focus from the
fg tab's textarea.

Reopening.

Comment 37

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Testcase for regression in comment 6/comment 10

Here's a testcase for the regression in comment 6/comment 10.  If you've got a
fast connection, you might have to find a longer/slower-loading bug than bug
124750 in order to get your focus back into the <textarea> and start typing.

(The STR in comment 10 are better in that regard, since there's a form
submission, an interstitial page, and then rendering the new page taking place
between the beginning of the process and the final jump-to-anchor.  And it's a
valid real-world scenario.  But this testcase is simpler.)

Comment 38

[Mark Mentovai]

I don't see the docshell's GetVisible() being called for the "winning" doc when
focus is lost.

Comment 39

[Asa Dotzler [:asa]]

Mark, so we've taken both patches and they're not working? Or we've only taken
one and it's not working?

Comment 40

[Boris Zbarsky [:bzbarsky]]

Asa, as far as I can tell we've taken both patches and they're working when the
code they changed is called.  But there is another Gecko bug involved which is
that we're not calling the code this bug changed in some cases when we should. 
Which by the way means that bug would show up in Firefox as well, I would
assume.  I would suggest this bug be reresolved and that we file a NEW bug on
this separate issue.

Comment 41

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #40)
> But there is another Gecko bug involved which is
> that we're not calling the code this bug changed in some cases when we should. 
> Which by the way means that bug would show up in Firefox as well, I would
> assume.  

That's not true, at least for this case (even before the checkins for this
bug)--see again comment 10.  This is a Camino-only regression from 1.7-branch
behavior.

Comment 42

[Boris Zbarsky [:bzbarsky]]

So it sounds like this camino bug covers at least two separate issues, since
some testcases are fixed and others are not...

Comment 43

[Mark Mentovai]

(In reply to comment #39)
> Mark, so we've taken both patches and they're not working? Or we've only taken
> one and it's not working?

We've taken both and it's fixed the bug most of the way - a background tab can
no longer purposely steal focus away from a foreground tab.  This is what most
of the recent comments discussed and what the initial "testcase" on this bug
demonstrated.  However, when a background tab finishes loading, it still will
steal focus.  It's a subtly different bug.

Comment 44

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #43)
> However, when a background tab finishes loading, it still will
> steal focus.  It's a subtly different bug.

Just to clarify further, "finishes loading and jumps to an anchor"; 'regular'
pages in bg tabs won't, which is why in comment 10 I thought it might have been
aaronl's checkin for bug 258514 that caused this regression.

Comment 45

[Asa Dotzler [:asa]]

OK. What's left isn't going to block us, I think.

Comment 46

[Samuel Sidler (old account; do not CC)]

The checkin for this bug may have caused the regression in bug 313811.

Comment 47

[Samuel Sidler (old account; do not CC)]

Comment on attachment 200036 [details] [diff] [review]
Camino part of patch

This was backed out to fix the regression in bug 313811.

Comment 48

[Mike Pinkerton (not reading bugmail)]

annoying, but going to require more work than we can do for 1.0

Comment 49

[Nat Tuck]

Shouldn't the structure of the code that deals with keyboard inputs be such that inactive tabs simply have nothing to do with the actual input focus?

Comment 50

[Samuel Sidler (old account; do not CC)]

This wasn't really fixed way back when since it was backed out to fix a regression.

It'd be great to know if the embeddor issues with the patch in bug 124750 will be addressed in Gecko 1.9.

Comment 51

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

No, the patch to the Core fixed the testcase in comment 11 (aka the embedding version of bug 124750) even after the 18branch-only backout of the Camino part.

The testcase in comment 37 (for the regression this bug was filed about, jumping to an anchor in the bg steals focus) has never been addressed.  Since it doesn't happen in Fx, presumably it's another embedding bug (of a similar class) lurking. 

Comment 52

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

The testcase in comment 37 seems to work (properly keep focus) sometimes now, which is annoying....  Maybe there's now a timing component to the bug, too?

(And please note again this bug got hijacked to fix another, related but separate bug ;) but not the original regression in comment 6/comment 10 that the testcase in comment 37 covers.  It's highly unlikely any fix will be branch-safe at this point :( so moving this to Cm2.0...)

Comment 54

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I just had this (jumping to an anchor in a bg tab) actually focus the bg window in which the tab was loading/jumping, making that window frontmost :(

Comment 55

[Richard van den Berg]

I still come across this bug daily using Firefox 2.0.0.6 on Windows XP. I'll have a slimserver page in a window or tab while writing a message in SquirrelMail. When the slimserver page loads, the focus of the textarea of the SM compose window is gone. Very annoying.

Comment 56

[Boris Zbarsky [:bzbarsky]]

This is a Camino bug.  Whatever you're seeing with Firefox is a different issue.

Comment 57

[Damon Sicore (:damons)]

-'ing per comment #56.

Comment 58

[Samuel Sidler (old account; do not CC)]

Damon, I think you misinterpreted comment 56. I believe Boris was saying this bug is about an issue seen in Camino but not one seen in Firefox. Per the patches in this bug that were initially landed, this is probably a problem with the docshell (Boris, correct me if I'm wrong here).

See also my comment 50.

Re-requesting blocking.

Comment 59

[Boris Zbarsky [:bzbarsky]]

The docshell patches landed for this bug are still in the tree, no?  It's just the camino part that got backed out.

Frankly, I think the two issues in this bug should have been split up into separate bugs.  At this point, I have a very hard time telling what, if any, core problems are left.

See also comment 51 and comment 40.  This isn't going to go anywhere till someone with Camino sits down with a debugger and sees what's up.

Comment 60

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

I really hate this bug, but until we get some developer-cloning working, it's not a 2.0 bug anymore.

At some point when I have time, I'll split out the comments pertaining to the comment 6/comment 10 regression into a new bug and morph this one into what it actually fixed (comment 14 et seq.), but anyone who has the time is welcome to do it before I get to it.

Comment 61

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Like bug 462099, this appears to be fixed in 1.9.2; we can mark it so if we get official 1.9.2 nightlies.

Comment 62

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #61)
> Like bug 462099, this appears to be fixed in 1.9.2; we can mark it so if we get
> official 1.9.2 nightlies.

WFM in 1.9.2 nightlies.