screen distorts after tapping on any search suggestion on some devices

VERIFIED FIXED in Firefox 26

Status

()

VERIFIED FIXED
5 years ago
2 years ago

People

(Reporter: krudnitski, Assigned: mcomella)

Tracking

26 Branch
Firefox 26
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox26+ verified, firefox27 unaffected, firefox29 unaffected, relnote-firefox 26+, fennec26+)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
On my Nexus 7 using GA code (Fx 26).

Start typing in the search bar (any ol' search term, like 'boy'). Tap on a search suggestion (any ol' search suggestion, like 'boy names').

The screen distorts before displaying results. Every time. It's awful. Truly truly awful.
This sounds like Bug 935604 - Regression: Graphical corruption shown on screen during page load
I can repro using the Nexus 4 and the effect is similar to bug 935604. I wonder if the second patch that wasn't uplifted there accidentally fixed this.

Finkle, do you think this is worth fixing/investigating, at the very least to see if the second patch from bug 935604 would fix this (since it's baked a while on the other channels)?
status-firefox26: --- → affected
status-firefox27: --- → unaffected
status-firefox29: --- → unaffected
Flags: needinfo?(mark.finkle)
First, let's get Karen (or someone) to verify that Fx27(Beta) is not affected.

Then we can ask for approval to land on release and determine if a Fx26.0.1 will be released. Or if this bug is bad enough to cause us to ship a Fx26.0.1 for mobile.
Flags: needinfo?(mark.finkle)
(Reporter)

Comment 4

5 years ago
I installed fx27 / beta and can't reproduce it.

It is an awful experience to hit, especially on a GA release. I would advocate for a respin given the number of affected devices.
FYI: Me, too. Confirmed repro case with Fx26 GA but not Fx27b1.
Mike - OK, sounds like we need an uplift patch, and request for uplift to release. Then we can get RelMan involved too. 

Lukas - We have a possible Fx26.0.1 candidate.
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(lsblakk)
I'm confused here.

In https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c51 we mentioned on 26 that this was still broken and then in https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c54 it was toggled to fixed.
tracking-firefox26: --- → +
Flags: needinfo?(lsblakk)
This affects a significant amount of devices, is easily reproducible, and is a new regression to FF26 - given all that we must do a 26.0.1 in order to never ship this to our users.  An email has been sent to release-drivers and we're hoping to kick builds off in the next couple of hours once a patch has been verified and uplifted to mozilla-release branch.  Please ping in IRC for approvals so we don't waste any time.
(In reply to Aaron Train [:aaronmt] from comment #7)
> I'm confused here.
> 
> In https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c51 we mentioned on
> 26 that this was still broken and then in
> https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c54 it was toggled to
> fixed.

There were multiple STR on bug 935604. In particular, there is:
  1) Going to any page through the search engine screen
  2) Going from about:home to about:home (having to actually type it in via the url bar to get the search engine screen)

(1) Was fixed by patch #1, uplifted to 26, while (2) was fixed by patch #2, uplifted only to 27 since it didn't seem to be a common use case.

Now, this bug is STR #3, which I didn't realize existed. I think patch #2 coincidentally fixes the issue though I will investigate further.
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
tracking-fennec: ? → 26+
(In reply to Michael Comella (:mcomella) from comment #10)
> (In reply to Aaron Train [:aaronmt] from comment #7)
> > I'm confused here.
> > 
> > In https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c51 we mentioned on
> > 26 that this was still broken and then in
> > https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c54 it was toggled to
> > fixed.
> 
> There were multiple STR on bug 935604. In particular, there is:
>   1) Going to any page through the search engine screen
>   2) Going from about:home to about:home (having to actually type it in via
> the url bar to get the search engine screen)

I see. If I had to guess on why this was missed on 26 is because we tested largely #1 with the patches that did land. I'm not entirely sure why we didn't see this search result issue too but we'll convene and talk about it.
Created attachment 8350200 [details] [diff] [review]
Remove graphical corruption when tapping Google search suggestion.

I followed the FF 27 patch #2 from bug 935604 very closely [1].

The important things that changed in that patch were:
  0) Change hideBrowserSearch to manage the HomePager visibility, rather than
keeping that code in filterEditingMode and dismissEditingMode (why? notice that
commitEditingMode is missing here).
  1) hideBrowserSearch should get called before hideHomePager. hideHomePager
sets the Layer View visibility but returns instantly if it's not visible.
Therefore, we call hideBrowserSearch first to reset the HomePager visibility
(after change #0 occurs) so it can properly set the Layer View visibility. The
corruption occurred because the LayerView was not present.

This patch changes from the FF 27 patch in that animateHideHomePager is a
thing. As it turns out, the animation is TODO, so all FF 26 hideHomePager (with
and without animations) are equivalent to the FF 27 hideHomePager methods. See
FF 27 [2] vs FF 26 [3]. Notice the animate parameter is unused.

I removed the first animateHideHomePager in commitEditingMode because it
returns instantly as the HomePager is not visible. This way, it also better
matches the FF 27 patch.

[1]: https://bugzilla.mozilla.org/attachment.cgi?id=8334185&action=diff
[2]: https://mxr.mozilla.org/mozilla-beta/source/mobile/android/base/BrowserApp.java#1650
[3]: https://mxr.mozilla.org/mozilla-release/source/mobile/android/base/BrowserApp.java#1627
Attachment #8350200 - Flags: review?(sriram)
Comment on attachment 8350200 [details] [diff] [review]
Remove graphical corruption when tapping Google search suggestion.

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

Sounds good.
Attachment #8350200 - Flags: review?(sriram) → review+
(In reply to Aaron Train [:aaronmt] from comment #11)
> I see. If I had to guess on why this was missed on 26 is because we tested
> largely #1 with the patches that did land. I'm not entirely sure why we
> didn't see this search result issue too but we'll convene and talk about it.

I should have realized that I was the only one who really understood all the different STR and their connections with one another, and then explained it better. Sorry about that.
Created attachment 8350217 [details] [diff] [review]
Remove graphical corruption when tapping Google search suggestion.
Attachment #8350200 - Attachment is obsolete: true
Created attachment 8350218 [details] [diff] [review]
Remove graphical corruption when tapping Google search suggestion. v2

There was a bug where the LayerView would not reappear when dismissing (no
commit) the HomePager on a page besides about:home.

I fixed this by reversing the order of hideBrowserSearch and hideHomePager in
dismissEditingMode (things to change #1 in comment 12). This also matches what
happens in release where cancelEdit eventually calls onStopEditing [1]).

Why this works correctly in 27 is beyond me but I will investigate to ensure we
don't have problems there.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#504
Attachment #8350218 - Flags: review?(sriram)
Attachment #8350217 - Attachment is obsolete: true
tried (In reply to Michael Comella (:mcomella) from comment #10)
> (In reply to Aaron Train [:aaronmt] from comment #7)
> > I'm confused here.
> > 
> > In https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c51 we mentioned on
> > 26 that this was still broken and then in
> > https://bugzilla.mozilla.org/show_bug.cgi?id=935604#c54 it was toggled to
> > fixed.
> 
> There were multiple STR on bug 935604. In particular, there is:
>   1) Going to any page through the search engine screen
>   2) Going from about:home to about:home (having to actually type it in via
> the url bar to get the search engine screen)
> 
> (1) Was fixed by patch #1, uplifted to 26, while (2) was fixed by patch #2,
> uplifted only to 27 since it didn't seem to be a common use case.
> 
> Now, this bug is STR #3, which I didn't realize existed. I think patch #2
> coincidentally fixes the issue though I will investigate further.

FWIW, i can't reproduce this on a Samsung Galaxy S3 device; Android 4.1.2.  Running Dec 5th build, FENNEC_26_0_RELEASE.  Tried all the different steps above (search suggestions, about:home), and the content will load properly as expected.   Must be hardware specific?
Attachment #8350218 - Flags: review?(sriram) → review+
(In reply to Tony Chung [:tchung] from comment #17)
> FWIW, i can't reproduce this on a Samsung Galaxy S3 device; Android 4.1.2. 
> Running Dec 5th build, FENNEC_26_0_RELEASE.  Tried all the different steps
> above (search suggestions, about:home), and the content will load properly
> as expected.   Must be hardware specific?

The original bug, bug 935604, was only reproducible on the N4 and N7 (if I'm not missing any). In every case, we're not displaying a view and thus we're displaying an empty graphics buffer (presumably, depending on Android's implementation), and I imagine the drivers on the Nexus devices crap up the buffer, while other devices do not.

I think it might just be LG Nexus devices as I cannot repro on a Samsung Galaxy Nexus.
Flags: needinfo?(michael.l.comella)
Comment on attachment 8350218 [details] [diff] [review]
Remove graphical corruption when tapping Google search suggestion. v2

[Approval Request Comment]
Regression caused by (bug #):
  Unknown

User impact if declined:
  Users on certain Nexus devices (N4, maybe N7, more?) will see graphical corruption when selecting a search suggestion, or typing about:home into the url bar while on about:home
 
Testing completed (on m-c, etc.):
  Similar patches have been on 27-29, for a while.
 
Risk to taking this patch (and alternatives if risky):
  We play with the web content and HomePager visibility. In the worse case, one of those does not reappear and we brick the browser, though it'd likely be fixed on force close restart.

String or IDL/UUID changes made by this patch:
  None
Attachment #8350218 - Flags: approval-mozilla-release?
Attachment #8350218 - Flags: approval-mozilla-release? → approval-mozilla-release+
Local build if anyone would like to test it before it gets merged into release:

http://people.mozilla.org/~mcomella/apks/mcomella-951776_01.apk
We're changing how the interaction of visibility between the different Views works, namely the about:home screen (aka the HomePager), the web content (aka the LayerView), and the search suggestions screen. As such, we should probably verify any combination of switching between these three views. Examples:
  1) Clicking the url bar and backing out of the about:home page with content typed
  2) Clicking the url bar and backing out of about:home without content typed
  3) Doing 1-2 but committing instead
  4) Doing 1-3 from both about:home and some page with web content
  5) Making sure the interaction between closing tabs w/ different views (i.e. about:home to web content and vice versa) works correctly

Additionally, I've only tested locally on an N4. We should also test an N7, probably an N5, and a few other devices (granted, less thoroughly) to ensure there are no issues.

I'll followup if I think of anything else.
Also verify that all the ways to leave the about:home screen work correctly:
  * Bookmarks
  * Top sites (thumbnails and list items)
  * History
  * Reading List
  * Tapping Search suggestions
  * Commiting a typed url
  * Selecting a history item in search suggestions
Verify:
  * Pressing back/forward to get to the about:home screen
Tested with no issues on Galaxy Nexus.

Note that on devices that the original issue cannot be repro'ed on (e.g. Galaxy Nexus), the testing probably does not need to be as thorough. However, it may be good to try a slower device to check for race conditions.
The above patch and landing has broken switch-to-tab content viewing. See http://www.youtube.com/watch?v=ACA5tRK57zk

Please fix and or back-out.
Created attachment 8350361 [details] [diff] [review]
Show web content when switching to the current tab.

There was a bug when on a page with web content, hitting the url bar, typing in
a query that would allow you to select the currently selected tab, and
selecting that tab will cause the web content not to reappear.

We needed to call hideHomePager after hideBrowserSearch in maybeSwitchToTab
also, not just in dismissEditingMode. The first hideHomePager needs to be
removed because the HomePager is actually visible when this line gets called,
resulting in the HomePager being hidden and the LayerView displayed.
hideBrowserSearch then shows the HomePager again, but I'm honestly not sure why
a broken HomePager is shown instead of a working one (perhaps some code that
runs later).

This ordering follows what's in Nightly [1], where mBrowserToolbar.cancelEdit()
eventually calls hideBS and hideHP in that order.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#1343
Attachment #8350361 - Flags: review?(sriram)
Attachment #8350361 - Flags: review?(sriram) → review+
Comment on attachment 8350361 [details] [diff] [review]
Show web content when switching to the current tab.

[Approval Request Comment]
Regression caused by (bug #):
  This one. :(

User impact if declined:
  Users who switch to the currently selected tab will not see web content until they switch to a page with about:home, open a new tab, or click the url bar
 
Testing completed (on m-c, etc.):
  Local
 
Risk to taking this patch (and alternatives if risky):
  Medium - It's just reordering a few statements, similar to the first patch, however, the worst case has the same issues as described in comment 19.

String or IDL/UUID changes made by this patch:
  None
Attachment #8350361 - Flags: approval-mozilla-release?
Comment on attachment 8350361 [details] [diff] [review]
Show web content when switching to the current tab.

Alright let's hope this forward fix addresses the issue.
Attachment #8350361 - Flags: approval-mozilla-release? → approval-mozilla-release+
Build in comment 28 was outdated up until now, sorry about that.
To be explicit, verify:
  * Switch tab functionality, including switching to the currently selected tab.
Patch #2 is actually a separate issue also present in 27: bug 952336 was filed for that.
Using the aforementioned and newest available local build and against the build based on 1445a7e883c5 (https://tbpl.mozilla.org/?tree=Mozilla-Release&rev=1445a7e883c5).

Based upon my personal testing tonight on a handful of manually ran test-cases

✓ = no visible graphical glitch, and function works as intended

LG Nexus 4 (Android 4.4.2) & LG Nexus 5 (Android 4.4.2) & Asus Nexus 7 (2013, Android 4.4.2) & Samsung Galaxy Note II (Android 4.3) (black-screen buffer)

↳ Tapping a search result after inputting text → ✓ (original reproducible issue here on mozilla-release)
↳ Entering editing mode and inputting a standard URL → ✓
↳ Selecting a history item in search suggestions in edit-mode → ✓ 
↳ Inputting 'about:home' from 'about:home' through the URL-bar → ✓
↳ Backing out of edit mode with content typed back to about:home → ✓
↳ Backing out of edit mode with no content typed back to about:home (no keyboard) → ✓
↳ Switching between open tabs with content → ✓
↳ Navigating from a bookmark from within about:home → ✓
↳ Navigating from a history item within about:home → ✓
↳ Navigating from a reading list item within about:home → ✓
↳ Navigating to a top-site item from thumbnails in about:home → ✓
↳ Navigating to a top-site item from the list of items in about:home → ✓ 

Devices I was not able to reproduce this bug on: Samsung Galaxy SIV (Android 4.3), HTC One (4.4), Samsung Galaxy Tab 3 10" (Android 4.2.2), , Samsung Galaxy SII (Android 4.1.2), Samsung Nexus S (Android 4.1), Asus Nexus 7 (2012, Android 4.4.2), Samsung Nexus 10 (Android 4.4) via :mschifer (see #QA backlog)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox26: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Updating summary to reflect the findings
Summary: screen distorts after tapping on a Google search suggestion → screen distorts after tapping on any search suggestion on some devices

Comment 37

5 years ago
Tested on LG Optimus 4X (4.1.2) using steps from comment35 I was not able to reproduce this issue
Based on exploratory testing, steps from comment35 and smoke-tests using LG Nexus 4 (Android 4.2.2) I haven't found any graphical issues.

Comment 39

5 years ago
Tested on Samsung Galaxy Tab 2 (4.1.1), using steps from comment35 and I was not able to reproduce the reported issues

Updated

5 years ago
Status: RESOLVED → VERIFIED
status-firefox26: fixed → verified
relnote-firefox: --- → 26+
You need to log in before you can comment on or make changes to this bug.