461403 - Page finishing loading in active tab of background window forces that window frontmost

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

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

From bug 359429:

--- Comment #15 From  Smokey Ardisson (no bugmail - do not email)   2008-10-22 15:03:13 PDT ---

I'm actually seeing this, or something like this (and not 306245, since no
anchors are involved), a lot in 2.0a1.  In fact, when Camino's no longer the
active app, the window-where-the-tab-finished-loading will pop to the front,
inactive.

Not sure if we want to open a new bug on trunk-only issues (since comment 11 is
definitely 18branch-only and moving to the front when the inactive app is new,
too). 

Or, maybe--since this clearly didn't use to happen on trunk--this is another
wacky regression from the keyboard loop, like bug 459254?  Sean, might your fix
for that bug help?

--- Comment #16 From Chris Lawson 2008-10-22 15:25:45 PDT ---

(In reply to comment #15)
> I'm actually seeing this, or something like this (and not 306245, since no
> anchors are involved), a lot in 2.0a1.  In fact, when Camino's no longer the
> active app, the window-where-the-tab-finished-loading will pop to the front,
> inactive.

Yeah, I've been meaning to file this for about two weeks now. It's driving me
crazy, and it happens pretty much all the time.

---

Filing this since it's a separate, trunk, regression--and, no, the patch for bug 459254 does not help.

In my experience it's far more common and far more annoying than bug 359429 was on the branch, so I think this needs to be fixed for 2.0.

Simple STR:

1. On a fresh profile, open the Tech News tab group.
2. Open a new window
3. Wait for cnet to finish loading in the active tab of the first (bg) window.

ER: Current (second) window remains frontmost/active
AR: Background (first) window becomes frontmost/active

We do need to drill down and find the exact regression range, in case it was a Gecko change that caused it or whatever.

Comment 1

[philippe (part-time)]

Works
Version 2.0a1pre (1.9.0.3pre 2008091900)

Cnet jumps in front and hurts me
Version 2.0a1pre (1.9.0.3pre 2008092000)

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=2008-09-19+00%3A00%3A00&maxdate=2008-09-20+00%3A00%3A00&cvsroot=%2Fcvsroot

Hmm, the tab loop is in there

Comment 2

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

Yeah, this is exactly the same situation as bug 459254: we have the keyboard loop stuff and bug 395423.

[8:25pm] cl: sauron: re bug 461403...I have a theory for you to try if you want
[8:26pm] cl: the line |    [browserWin setSuppressMakeKeyFront:NO];| occurs twice in MainController as well, in circumstances similar to what murph describes in the other bug.
[8:27pm] cl: remove those two lines and re-compile, and I bet the jumping focus bug goes away
[8:44pm] cl: hrm. so much for hunches. that didn't help, unfortunately.

Comment 3

[Sean Murphy]

Alright I found the problem thank goodness!  My key view loop patch definitely caused this, and here is why:

When we are finished loading a page, if the window is not key we cause the window to make the visible CHBrowserView it's first responder (the else clause below):

> - (void)loadingDone:(BOOL)activateContent
> {
>   // As Gecko juggles its internal views during page load, focus may fall back
>   // to the window; if that has happened, re-focus the content area.
>   if ([[self window] firstResponder] == [self window])
>     activateContent = YES;
>   if (activateContent) {
>     // if we're the front/key window, focus the content area. If we're not,
>     // set gecko as the first responder so that it will be activated when
>     // the window is focused. If the user is typing in the urlBar, however,
>     // don't mess with the focus at all.
>     if ([[self window] isKeyWindow]) {
>       if (![self userChangedLocationField])
>         [mBrowserView setBrowserActive:YES];
>     }
>     else
>       [[self window] makeFirstResponder:[mBrowserView browserView]];
>   }

And, my patch changed CHBrowserView to actually support becoming the key view, and when that happens, Gecko is activated (focusing the window):

>- (BOOL)becomeFirstResponder
>{
>  [self setActive:YES];
>  return YES;
>}

Basically, this can be fixed by only setting Gecko active, in becomeFirstResponder, if we are the key view.

Comment 4

[Sean Murphy]

Attachment: Patch

Fix using the above approach.

Note, this also takes care of bug 459254, so I'm canceling the "workaround" approach I was doing there, since this fixed the actual root cause of that bug and this one.

Comment 5

[Stuart Morgan]

Comment on attachment 344635 [details] [diff] [review]
Patch

Awesome; this makes me a lot more comfortable than messing with the suppression code. r=me.

Comment 6

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 344635 [details] [diff] [review]
Patch

sr=pink

Comment 7

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

Landed on cvs trunk!