170811 - after following link from iframe which loads in parent frame, focus is lost

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[sairuh (rarely reading bugmail)]

found using 2002.09.24.08 comm trunk builds (also a problem on mozilla).

after i follow a link in the tinderbox popup (associated with someone's
checkin), focus is lost in the resulting page.

i don't think this is a regression from the fix for bug 141295, as i remember
this was still an issue before the fix there was checked in.

1. go to http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey
2. click on a user's link in the "Guilty" column.
3. in the resulting pop-up, click on any of the first 3 links, which are
redirects to bonsai queries. eg, i often click "Check-ins within 24 hours"
4. wait for the bonsai query page to finish loading, then attempt to
scroll/navigate in the content area with the keyboard.

results: focus is lost, and unable to navigate the content area with the
keyboard. space, pageUp/Down, arrow keys do nothing --even hitting the tab key
doesn't restore/display focus (it's not in the urlbar either). need to
mouseclick to set focus.

Comment 1

[sairuh (rarely reading bugmail)]

side note: chimera does not exhibit this. not sure if that's coz chimera is on a
branch or a cocoa app...

Comment 2

[sairuh (rarely reading bugmail)]

okay, moz1.0 didn't have this (because ns7.0 doesn't show this). will narrow down...

Comment 3

[sairuh (rarely reading bugmail)]

looks like this regressed btwn the moz1.0 (fine) and moz1.1 (broken) releases.
sorry about the wide interval, but i don't have any trunk builds prior to
2002.06.10 (where it's broken).

Comment 4

[Brian Ryner (not reading)]

the 1.0 branch was created on 4/9/2002, so there's about a 2 month checkin
window.  Most likely suspects:

aaronl, bug 130969, 5/16/2002
bryner, bug 138237, 6/6/2002

Comment 5

[sairuh (rarely reading bugmail)]

since the fix for bug 138237 went into the moz1.0 branch as well as the trunk,
would it still be considered suspect?

Comment 6

[Dave Barrowman]

Topembed+ assuming it is in Gecko; if it is an app-level problem, topembed does
not apply.

Comment 7

[sairuh (rarely reading bugmail)]

this bug does occur with a *trunk* chimera test build that bryner whipped up
(from last month). so, yeah, i think it is a gecko issue.

Comment 8

[Brian Ryner (not reading)]

The regression isn't from either of the bugs I mentioned.  Guess I'll need to
pull some trees by date.

Comment 9

[Brian Ryner (not reading)]

jrgm sees this in a linux build from 4/25/2002, so that narrows the window
considerably.

Comment 10

[Brian Ryner (not reading)]

possible causes in this checkin window:

- jag, bug 37638 / bug 89835, 4/10/2002  [unlikely, unless Chimera-specific code
has the exact same bug]
- joki, bug 78989, 4/14/2002
- bryner, bug 106123, 4/15/2002
- bryner, bug 88239, 4/23/2002 (same deal as jag's checkin)

also could be related to <iframe> checkins:

bug 127891
bug 138676
bug 52334

Comment 11

[Brian Ryner (not reading)]

I narrowed this down to checkins between 4/16/2002 08:00 and 4/17/2002 08:00. 
It's almost definitely jst's checkin for iframe loading (bug 52334).

Additionally, the problem happens any time focus is in an iframe and a link is
followed.  For example, if I load tinderbox and click on the link inside the
ports iframe, focus is dead.  Updating the summary to reflect this.

jst, any ideas here?  I'm particularly interested in what might have changed in
regards to destruction of iframes.

Comment 12

[Brian Ryner (not reading)]

Attachment: patch

The changes in iframe loading require some changes in the logic here. 
Basically, since the iframe element now owns the frame loader, the iframe's
document can hang around past frame destruction (until a js gc, even).	So,
that's no longer a good way to detect that we're replacing the document in a
parent window of the focused window.  I've implemented the following approach:

- walk up the document chain starting with the document for the focused dom
window
- stop when we hit
  a) the top of the document chain.  this means the focused window is not a
child of the window we're loading into and focus should be unaffected.
  b) a document with no window.  this can mean that the dom window is now
showing another document, and we should go ahead and take focus.
  c) the dom window that this document is loading in.  this means that we are
replacing a document in a parent of the focused window and we should also take
focus.

I tested this on windows and linux and found no problems on either platform.

Comment 13

[saari (gone)]

Comment on attachment 119225 [details] [diff] [review]
patch

sounds reasonable enough, as long as jst agrees the new logic is sound. r=saari

Comment 14

[Johnny Stenback (:jst)]

Comment on attachment 119225 [details] [diff] [review]
patch

sr=jst

Comment 15

[Brian Ryner (not reading)]

fix checked in.

Comment 16

[sairuh (rarely reading bugmail)]

brian, thanks so much for investigating and fixing this.

vrfy'd fixed with 2003.04.03 comm trunk builds (win2k, mac os 10.2.4, linux rh8.0).

Comment 17

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381