383821 - Focus lost after loading page by following link

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect)

Description

[philippe (part-time)]

STR
1. load http://www.guardian.co.uk
2. follow a link to an article
3. page loaded, use spacebar/arrowkeys to scroll down
result: computer beeps, no scrolling.

The page is not focussed at all. This is not the same as those bugs where the focus is stuck on the back button.
Clicking in the page restores the focus.

I've reproduced this on a number of sites whose pages contain iframes.
with or without adblocking (on the Guardian site, all ads are normally blocked on my side).

Minefield builds don't have this problem.

Alternative STR:
1. View source on a page
2. Cmd-A to select all code.
Result: Nothing is selected.

Regression range:
works: Version 2007053002 (2.0a1pre)
fails: Version 2007053112 (2.0a1pre)

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-05-30+02%3A00%3A00&maxdate=2007-05-31+12%3A00%3A00&cvsroot=%2Fcvsroot

bug 373122 ? or bug 380465 ?

Comment 1

[Chris Lawson (gone)]

I've seen this a few times recently too, though I haven't tried to reproduce it. I certainly see this specific instance of it and can reproduce.

Comment 2

[philippe (part-time)]

A few more observations:

1. With Full Keyboard Access turned ON, the focus is actually on the bookmark bar (pressing tab will then focus the first entry in the bookmark bar). With FKA turned OFF, pressing tab goes to the first link on the page - but loading a page as outlined in comment 0, and pressing the space-bar doesn't scroll the page.
That is somehow similar to bug 185385, although I never experienced that one.

2. If the page is loaded from bookmark or accessed by inputting the url in the location-bar, then the focus is correctly set and pressing space-bar allows for scrolling (or the page is accessed by clicking a link in another app - e.g feed reader).

3. I can only reproduce this with pages that contain an iframe, and only with a trunk build; 1.6a1pre is not affected.

Comment 3

[philippe (part-time)]

Attachment: testcase

Follow the link and press space bar: no scrolling.
Copy paste the url into the location bar, press return, press space bar: works correctly.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

I'm able to reproduce the symptoms described in comment #0 (I haven't
yet tried to reproduce those in comment #2 and comment #3).

I've also found that these problems (at least those in comment #0) go
away when I make the following change to the patch for bug 373122 (the
changed code is from nsChildView::TearDownView() in
widget/src/cocoa/nsChildView.mm, and partially rolls back the patch):

 if ([mView isEqual:[win contentView]]) {
   [mView release];
 } else {
   // Stop NSView hierarchy being changed during [ChildView drawRect:]
-  [mView performSelectorOnMainThread:@selector(delayedTearDown) withObject:nil waitUntilDone:false\
];
+  [self removeFromSuperviewWithoutNeedingDisplay];
+  [self release];
 }

I'm _very_ puzzled by this.

This part of my patch (the one I reversed in the above code) is needed
to stop a nasty problem happening when a ChildView object (a subclass
of NSView associated with an nsChildView Gecko object) is removed from
its superview during a call to [ChildView drawRect:].  The call to
mGeckoChild->DispatchWindowEvent(paintEvent); in [ChildView drawRect:]
sometimes (indirectly) calls ~nsChildView::nsChildView() (and
nsChildView::TearDownView()).  My workaround is to postpone removing
the ChildView object from its superview until after the call to
nsChildView::TearDownView() is finished (thereby guaranteeing that it
never happens during a call to [ChildView drawRect:]).  I could find a
different workaround (for example I could call [ChildView
delayedTearDown] only when nsChildView::TearDownView() has been called
from inside [ChildView drawRect:]).  But I can't simply undo my patch.

It looks like something in Camino doesn't like it when a ChildView
object isn't removed from its superview during the call to
nsChildView::TearDownView() (which is in turn called from the
nsChildView destructor).  Stuart, do you have any idea what that might
be?

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Here's the quote from my bug 373122 patch again, without the broken line:

 if ([mView isEqual:[win contentView]]) {
   [mView release];
 } else {
-  // Stop NSView hierarchy being changed during [ChildView drawRect:]
-  [mView performSelectorOnMainThread:@selector(delayedTearDown)
-         withObject:nil waitUntilDone:false];
+  [self removeFromSuperviewWithoutNeedingDisplay];
+  [self release];
 }

Comment 6

[Steven Michaud [:smichaud] (Retired)]

The same part of my bug 373122 patch also "causes" (i.e. triggers) bug
384343.  In both cases I suspect this part of my patch might have
unmasked other bugs.  Since I'm already working on Cocoa widgets, I'll
be looking for these "other bugs" in Cocoa widgets myself.  I'll
report back here if I find anything interesting.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think this is the same issue as a serious problem I'm having in GMail: advancing to the next or previous message in a "folder" causes the window to lose focus (I think because of an IFRAME load). This is horrible because it means you can't just read through a folder with the keyboard, you have to keep switching between the keyboard and the mouse to restore focus.

Comment 8

[Chris Lawson (gone)]

It looks to me that this is why you can't use FAYT in autostart mode in View Source pages anymore, too. You have to click in the page before FAYT will register any input. The Find dialog functions normally.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

> in autostart mode

Tell me what this means and I'll see if the problem goes away in my
build with my bug 373122 fix partially disabled.

(And by the way, are you talking about Camino only, or all the trunk
browsers?)

Comment 10

[Chris Lawson (gone)]

Just Camino, but only because I don't use any other trunk Gecko browsers.

FAYT = find-as-you-type; it has an autostart mode that allows it to begin searching when text is entered (instead of the normal activation with the / key). See 

http://kb.mozillazine.org/Accessibility.typeaheadfind.autostart

cl

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Yup, FAYT in a View Source page (with or without autostart mode) seems
to be another casuality of (the same part of) my bug 173122 patch --
in both Camino and Minefield on the trunk (I haven't tested Seamonkey).
You have to click in the page to get it to work.

Comment 12

[Steven Michaud [:smichaud] (Retired)]

I've just posted a patch at bug 384343 comment #6 that fixes the
problem reported there and those reported here (at bug 383821) ... at
least those that I can reproduce.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

I've just landed my patch for bug 384343 on the trunk, which also
fixes this bug.

Comment 14

[philippe (part-time)]

Thanks, Steven. Works nicely.
--> verified using Camino 2007062119 (2.0a1pre)