623624 - Content elements don't get focus when the focus method is set on them

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase1, focus input on load

See upcoming testcase. In all cases, the element's background-color should be green on load or immediately after load.
This doesn't happen with Fennec at the moment.

This is more or less the opposite of the summary mentioned in bug 532738. Fwiw, I don't agree with the current summary of that bug ("Don't auto-focus input fields on page load").

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase2, focus link on load

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase3, focus link after setTimeout of 0ms after load

Comment 3

[Martijn Wargers (dead)]

Attachment: testcase3, focus link after setTimeout of 0ms after load

Comment 4

[Martijn Wargers (dead)]

On first load, the elements don't get focus (FAIL), on reload they do get focus (PASS).

Comment 5

[Martijn Wargers (dead)]

Also, closing the tab, then undo close tab, makes the tests fail.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Testcase #3 works for me on Android. I suspect we are not allowing the content
process to become "active" soon enough after a load. Which is why the
setTimeout testcase and the reloads work.

Comment 7

[Martijn Wargers (dead)]

Attachment: testcase1, focus input on load, with some blur/focus event logging

Comment 8

[Martijn Wargers (dead)]

Attachment: testcase2, focus link on load, with some blur/focus event logging

Comment 9

[Martijn Wargers (dead)]

Attachment: testcase3, focus input after setTimeout of 0ms after load, with focus/blur event logging

Comment 10

[Martijn Wargers (dead)]

Attachment: testcase4, focus link after setTimeout of 0ms after load, with focus/blur event logging

Comment 11

[Martijn Wargers (dead)]

There is a hack in Fennec code that removes focus on page load, see bug 532738, comment 11.

What I get with the stock Android browser on these testcases:
testcase 1: input doesn't become green, but I get an 'input focus' event logged. When tapping on the input, the input becomes green and another 'input focus' event gets logged.

testcase 2: link doesn't become green, but I get an 'input focus' event logged. Tapping on the link doesn't make it green, no 'input focus' event gets logged. Note that you get this result, if the first testcase wasn't carried out before. 

testcase 3: In short, same results as testcase 1.

testcase 4: The same results as testcase 3.

Note that this happens when the content page doesn't seem to have 'real' focus. If the content page does have focus, then you get the results as expected (except perhaps, that you can't focus links by tapping on them)

It doesn't matter if the hardware keyboard is opened or closed. The software keyboard doesn't show up when the page script is calling focus.

Ideally, Fennec should behave as expected, so untrusted focus events (from content script) shouldn't cause softkeyboard to appear.

Comment 12

[Martijn Wargers (dead)]

Another case, where testcase 3 doesn't pass.
- Open testcase 3 in a new tab.
- Press reload on the newly opened tab with testcase 3.

Actually, I think this is one of the reasons why I see a lot of mochitest failures related to focus. If a tab/window, for some reason, doesn't have focus, there is no way to get it back (not by using the .focus() method, at least)

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch for activating the remote frame on tab switching

The activeRemoteFrame call has been added during the days we moved from a non-e10s world to an e10s world (bug 573041) and has survived in TapDown until now. I don't see a good reason for not moving this call to selectedTab?

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 514498 [details] [diff] [review]
Patch for activating the remote frame on tab switching

We need some test coverage on this ASAP.

Comment 15

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/e230a8add766

Comment 16

[Matt Brubeck (:mbrubeck)]

Backed out for a possible Ts regression:
http://hg.mozilla.org/mobile-browser/rev/01d357abf087

Comment 17

[Matt Brubeck (:mbrubeck)]

Talos results from the backout indicate that this patch probably caused a 100ms (2%) regression in Ts.

Comment 18

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

I'm unsure about the fact the regression is caused by browser.focus(); but it could be possible (strange things happened during startup!) so this patch remove this line

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

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

Make sure we watch for another regression

Comment 20

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

http://hg.mozilla.org/mobile-browser/rev/5d77401196ed

Comment 21

[Martijn Wargers (dead)]

Ok, verified fixed on Fennec trunk on the Droid (and Windows desktop).
This is fixed for the case mentioned in comment 12, not for the original issue that I filed.
But that will be fixed by the fix for bug 532738 afaics, so I won't file a new bug for this, I'll just add a comment in there.