584329 - a iframe-window set as the focusedWindow still has focus after the iframe is set hidden

Status: NEW

(Core :: XUL, defect)

Description

[Asaf Romano (gone)]

Found this while investigating bug 574353.

If a the window of a xul iframe is set as the focused window, and no element within it actually received focus, the iframe-window would still be marked as the focused window if the iframe is set hidden.

I cannot tell if this is a regression.

Comment 1

[Neil Deakin]

A simple test shows that no 'pagehide'/'unload' event fires at the frame's document when it is hidden in this way, so the focus manager isn't getting notified.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

What does "iframe is set hidden" mean? display: none? visibility: hidden?

In the first case we could detect that the iframe's rendering object is deleted,
but not sure what to with the latter case.

Comment 3

[Neil Deakin]

The former.

Bugs 570835 and 561940 are also related.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Would be great to have a testcase here.

So, if the case is display: none, nsSubDocumentFrame::DestroyFrom could
probably unfocus the window/element in the iframe.

Comment 5

[Johnathan Nightingale [:johnath]]

Neil continues to think this blocks. Throwing it his way.

Comment 6

[Neil Deakin]

Attachment: work in progress

One of the ime tests fails with this patch I think, so I need to investigate it.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 498223 [details] [diff] [review]
work in progress

>-[scriptable, uuid(5cb91200-53f6-4d35-989d-1d28ad80a0d4)]
>+[scriptable, uuid(F91024AD-F5A8-42CB-851E-57033458F69C)]
Can't take interfaces changes to trunk atm.

There is nsIFocusManager_MOZILLA_2_0_BRANCH , which could be perhaps used,
or do we need nsIFocusManager_MOZILLA_2_0_BRANCH_2 for this?
Perhaps bsmedberg could know that.



>@@ -248,19 +248,20 @@ interface nsIFocusManager : nsISupports
>    *
>    * If aNeedsFocus is true, then focus events are expected to be fired on the
>    * window if this window is in the focused window chain.
>    */
>   [noscript] void windowShown(in nsIDOMWindow aWindow, in PRBool aNeedsFocus);
> 
>   /**
>    * Called when a document in a window has been hidden or otherwise can no
>-   * longer accept focus.
>+   * longer accept focus. If aDestroyed is true, then the window is being
>+   * destroyed and focus should move elsewhere.
>    */
>-  [noscript] void windowHidden(in nsIDOMWindow aWindow);
>+  [noscript] void windowHidden(in nsIDOMWindow aWindow, in PRBool aDestroyed);
aDestroyed is strange name for the parameter. The window isn't being destroyed, only its
presentation.

Comment 8

[Neil Deakin]

It seems that the test_imestate.html fails with this patch because setting designMode="on" causes the document frame to be destroyed (and recreated perhaps?)

Any thoughts here as to why or how to work around this?

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Ah, interesting problem. And hard to resolve.
What does 3.6 do? Or 3.5 which doesn't have focusmanager, IIRC.


Btw, why is this a blocker?

Comment 10

[Neil Deakin]

(In reply to comment #9)
> Btw, why is this a blocker?

It probably doesn't.

Comment 11

[(no longer active)]

I think the right thing to do here is to let the drivers retriage this...

Comment 12

[Dietrich Ayala (:dietrich)]

Forecast is mildly annoying with easy workaround (focus something). Blocking-.

Please re-request blocking if you have a scenario which is bad enough that we should hold the release for a fix.

Comment 14

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Any updates?  We should fix this for whatever the latest release is..

Comment 15

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336