925012 - Search engine choices pane is completely blank

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Chenxia Liu [:liuche]]

Attachment: Screenshot: empty search suggestions

On the search engines state for the new about:home, we can reach a state where the search shortcuts pane is completely blank.

STR:
1. Type into search bar to bring up search shortcuts.
2. Scroll on the search shortcuts (this dismisses the keyboard).
3. Tap on the menu bar; this highlights the text and brings up the keyboard.
4. Start typing.

Expected:
Search shortcuts pop up to match the entered text

Actual:
Entire pane goes blank, see screenshot.

Note: Dismissing the keyboard via the back key and starting to type again does not cause this problem.

Comment 1

[Aaron Train [:aaronmt]]

I'm not able to reproduce, maybe others can? Sounds bad, tracking?

Comment 2

[:Margaret Leibovic]

I can't reproduce this either, but I'm seeing a terrible glitch when the keyboard dismisses where it looks like we draw the list of search suggestions twice momentarily :/

Comment 3

[Teodora Vermesan (:TeoVermesan)]

I am able to reproduce this on Samsung Galaxy Nexus (Android 4.1.1) with SwiftKey keyboard. I will try to find a regression range.

Comment 4

[Teodora Vermesan (:TeoVermesan)]

Regression window-wanted:

There are three builds in 08/21 central, when fig has been pushed to m-c. The first build is not affected and the second one is affected.
The pushlog is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d136c8999d96&tochange=b2486721572e

Comment 5

[Aaron Train [:aaronmt]]

... so new-about-home

Comment 6

[Lucas Rocha (:lucasr)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #4)
> Regression window-wanted:
> 
> There are three builds in 08/21 central, when fig has been pushed to m-c.
> The first build is not affected and the second one is affected.
> The pushlog is:
> http://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=d136c8999d96&tochange=b2486721572e

Do you mean that the first nightly with new-about-home was working fine? It would be nice to know what changed between the first and second builds.

Comment 7

[Teodora Vermesan (:TeoVermesan)]

The first build from 08/21 is unaffected and it is with the old about:home. The second build from 08/21 is the affected one, with the new about:home

Comment 8

[Lucas Rocha (:lucasr)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #7)
> The first build from 08/21 is unaffected and it is with the old about:home.
> The second build from 08/21 is the affected one, with the new about:home

Just to confirm: does this only happen with the SwiftKey keyboard?

Comment 9

[Teodora Vermesan (:TeoVermesan)]

(In reply to Lucas Rocha (:lucasr) from comment #8)
> Just to confirm: does this only happen with the SwiftKey keyboard?

Yes, I am not able to reproduce with other keyboards.

Comment 10

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

Jim - Are there any gotcha's with swiftkey we should be looking for? Can you look at the logs for any issues?

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

I did some logging. When typing to replace existing, selected text, SwiftKey deletes the selected text first, before committing new text. This is different from the stock keyboard behavior which commits the new text directly and relies on the editor to delete the old text.

For example, if the current text is "foo" and selected, SwiftKey will call DeleteText(0, 3) then InsertText("bar"), whereas the stock keyboard only calls InsertText("bar"). The result is the same but SwiftKey makes an extra call.

In SwiftKey's case, the text field is temporarily blank due to the DeleteText call, so maybe that messes up the search code.

Comment 12

[Brian Nicholson (:bnicholson)]

Attachment: Force execution of pending BrowserSearch fragment removals before re-adding

As Jim said, SwiftKey deletes the text before committing new text. This triggers hideBrowserSearch() since the text is empty, then showBrowserSearch() immediately after for the committed text.

When this happens, the container given to BrowserSearch#onCreateView is null. I assume this is an Android bug, and I filed it as http://code.google.com/p/android/issues/detail?id=61179. Calling FragmentManager#executePendingTransactions seems to work as a workaround.

Comment 13

[Lucas Rocha (:lucasr)]

Comment on attachment 818792 [details] [diff] [review]
Force execution of pending BrowserSearch fragment removals before re-adding

Review of attachment 818792 [details] [diff] [review]:
-----------------------------------------------------------------

Nice catch.

::: mobile/android/base/BrowserApp.java
@@ +1607,5 @@
> +        // (see bug 925012). Because of an Android bug (http://code.google.com/p/android/issues/detail?id=61179),
> +        // calling FragmentTransaction#add immediately after FragmentTransaction#remove won't add the fragment's
> +        // view to the layout. Calling FragmentManager#executePendingTransactions before re-adding the fragment
> +        // prevents this issue.
> +        getSupportFragmentManager().executePendingTransactions();

nit: getSupportFragmentManager() only once and use it for both calls:

final FragmentManager fm = getSupportFragmentManager();

fm.executePendingTransactions();
fm.beginTransaction()...

Comment 14

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/fx-team/rev/6cf99173cf98

Comment 15

[Brian Nicholson (:bnicholson)]

Comment on attachment 818792 [details] [diff] [review]
Force execution of pending BrowserSearch fragment removals before re-adding

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 862793
User impact if declined: search screen can be blank with SwiftKey keyboard (see screenshot)
Testing completed (on m-c, etc.): locally
Risk to taking this patch (and alternatives if risky): very low risk
String or IDL/UUID changes made by this patch: none

Comment 16

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6cf99173cf98

Comment 17

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 27.0a1 (2013-10-20)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1

Comment 18

[Aaron Train [:aaronmt]]

Please remember to flip status.

Comment 19

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/87072df529bb