348369 - Tabbrowser steals focus any time the current browser changes

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect, P3)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:

1) Start Firefox.
2) Set browser.tabs.loadDivertedInBackground to false.
3) Set browser.link.open_newwindow to 3.
4) Set browser.link.open_newwindow.restriction to 0.
5) Disable the popup blocker.
6) Run the javascript: URL in this URL field.
7) Focus some other app.
8) Wait 5 seconds.

ACTUAL RESULTS:  Other app loses focus, Firefox gets it

EXPECTED RESULTS:  Do not steal focus!

DETAILS:  The window.focus() call happens from 

2 setFocus(element = [object XPCNativeWrapper [object Window @ 0x8291c40 (native @ 0x94e3f24)]], 98) ["chrome://global/content/bindings/tabbrowser.xml":766]

We should probably just shift focus around without actually calling window.focus if we're not the active window.

NOTE: This makes running automated tests that include window.open() pretty painful.

Comment 1

[Asaf Romano (gone)]

Attachment: patch

Stolen from bug 232605.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

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

>Index: toolkit/content/widgets/tabbrowser.xml

>-            function setFocus(element) {
>-              document.commandDispatcher.suppressFocusScroll = true;
>-              try {
>-                element.focus();
>+ 

nit: remove one of the blank lines here.

>+                if (aElement instanceof Window) {
>+                  cmdDispatcher.focusedWindow = aElement;
>+                  cmdDispatcher.focusedElement = null;
>+                }
>+                else {
>+                  cmdDispatcher.focusedWindow = null;
>+                  cmdDispatcher.focusedElement = aElement;
>+                }

Why did you change it to use null for the focusedWindow in the second case?

Comment 3

[:Gavin Sharp [email: [e-mail]]]

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

Just moving this out of my queue until comment 2 is answered.

Comment 4

[Asaf Romano (gone)]

I shouldn't have.

Comment 5

[Asaf Romano (gone)]

Attachment: patch

Comment 6

[:Gavin Sharp [email: [e-mail]]]

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

I wasn't not able to test this, but r=me assuming you did.

Comment 7

[Asaf Romano (gone)]

mozilla/toolkit/content/widgets/tabbrowser.xml 1.214

Comment 8

[Ria Klaassen (not reading all bugmail)]

Could this bug have caused bug 360714?

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #8)
> Could this bug have caused bug 360714?

Pretty unlikely I think. Bug 130078 is more likely the cause of that regression, in my opinion.

Comment 10

[Benjamin Smedberg]

I don't know how we'd unit-test this, but I think we should find a way to do so.