91884 - should never call focus() directly in navigator.js (and other places?)

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[John Morrison]

Followon from bug 91788 "[mac] Parent window blanks out when a javascript popup 
window comes up"

In that bug, the fix was schedule a call to "_content.focus()" for later with
a setTimeout. For the branch, we only made that change for the specific case
where it was badly affecting the Mac (minimize the risk). However, as a welcome 
side effect of that change, this cleared up the problem where popup ads (and 
other window.open windows) would flash their outer frame on the screen at full 
browser size, and then show again at their specified (smaller) size.

It appears that we should never be calling focus() within the onload handler
of the browser (navigator.xul). {Maybe not in any XUL window). Calling focus
seems to cause this early show of the XUL window.

So this bug is to followup on that, and either put the other instances of 
_content.focus() on a timer (setTimeout), or figure out if there is some other
even better way to approach this problem.

Comment 1

[John Morrison]

This should be addressed (one way or another) for the 0.9.4 milestone.

Comment 2

[John Morrison]

setting 0.9.4 per earlier discussion.

Comment 3

[jag (Peter Annema)]

Attachment: [patch] Hack alert! Real fix is gonna be murder.

Comment 4

[jag (Peter Annema)]

Attachment: [patch] With comments.

Comment 5

[jag (Peter Annema)]

For now, since navigator's the only one running into it, and the real fix
consists of either postponing the focus from C++ till right after the window is
shown (which will temporarily cause a discrepancy between what the programmer
thinks has the focus, and what really has the focus), or making focus not show
the window (preliminary research on the related Windows APIs suggests this is
impossible), we've decided to go with this simple fix for now and deal with
implementing the "real" fix if/when it becomes necessary.

Other possible solutions include disabling focusing from onload (what ain't
there yet can't be focused), or documenting and evangelizing the fact that a
focus from onload will cause your window to display before onload is done (and
thus before some sizing code has been run), and should be avoided or postponed
if you don't want that to happen.

Comment 6

[John Morrison]

r=jrgm. That just extends the "hack" that shipped in 6.1 to cover all the cases
where focus() was being called onload in navigator.js.

I wonder, though, if there are any dialogs, or other main windows that could 
use this too.

Comment 7

[David Hyatt]

sr=hyatt.  open a new bug to track the generic solution and target it for
mozilla 1.0.

Comment 8

[jag (Peter Annema)]

Checked in the patch. Marking fixed.

Filed bug 96275.

Comment 9

[John Morrison]

May God have mercy on us all. The 212 bug spam-o-rama is Now!

Comment 10

[John Morrison]

*** Bug 95473 has been marked as a duplicate of this bug. ***

Comment 11

[Boris Zbarsky [:bzbarsky]]

should viewsource.js get a similar change?  that calls window._content.focus() 
onload....

Comment 12

[[e-mail]]

Style nit: should use setTimeout(_content.focus, 0);
instead of setTimeout("_content.focus();", 0);
etc.

Comment 13

[jag (Peter Annema)]

I'm not sure that will work, because you'll be passing in the focus() function
object without the _content context. Hence the "" around it, so it's eval'ed at
execute time. One option would be to do this:

setTimeout(function(element){element.focus();}, 0, _content);

This is kinda ugly though. But, we are working toward something similar to fix a
bug this code introduced (even though the code is correct, we're running into
another bug with our event system which we'll need to work around for now), see
bug 97067.