1565488 - Wrong panel from welcome screen is highlighted after clearing data

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: First Run, defect, P1)

Description

[Eliza Balazs]

Environment:
Device:

• Samsung Galaxy Tab S3 (Android 8);
• Huawei Honor 8 (Android 7.0).

Build:

• Nightly 68.1a1 (2019-07-10);
• Nightly 70.0a1 (2019-07-11).

Precondition: The user is logged in to Sync;

Steps to reproduce:

1. Clear Data from Android settings;
2. Launch Fennec and slide from the "Welcome" panel to the "Privacy" panel;
3. Observe the highlighted panel;

Expected result:
The "Privacy" panel is highlighted.

Actual result:
The "Welcome" panel remains highlighted, after sliding to the "Privacy" panel.

Notes:

• This is not reproducible on RC 68.0 build 3, Fennec ESR 68.1b1 build 2.
• Not reproducible when all the three panels are displayed, there is a slow change between the panels when sliding.
• Refreshing the page or going to Recent Apps solves the issue temporary.
• Video: https://drive.google.com/file/d/1GYFEXekj0SBwQQ8ed5zplypjXKZW0uIg/view

Comment 1

[BugBot [:suhaib / :marco]]

Change the status for beta to have the same as nightly and release.
For more information, please visit auto_nag documentation.

Comment 2

[Eliza Balazs]

Hi! This is also reproducible on ESR 68.1b3 with OnePlus 5T (Android 9).

Comment 3

[Brad Arant]

Was able to reproduce on Samsung Galaxy 8. Continuing resolution...

Comment 4

[Andrei Lazar]

Un-assigning myself since Brad is going to work on this.

Comment 5

[Brad Arant]

...

Comment 6

[Eliza Balazs]

Hi!
This is also reproducible with the following steps:

1. Clear data from Android settings;
2. Disable internet connection;
3. Launch Fennec.

Expected result:
"Welcome" panel is highlighted.

Actual result:
"Welcome" panel is not highlighted.

Notes:

• The blue line under the panel is visible.

Comment 7

[Brad Arant]

I am working on this and can recreate. Ill have fix shortly.

Comment 8

[Brad Arant]

So I have discovered the issue and here are my findings:
In the TabMenuStripLayout.java object when onPageSelected(position) is executed it sets a GlobalLayoutListener on the TabMenuStripLayout and for the most part this event is firing. On the two tabbed version of the FirstrunPager for some reason this event is never firing and the text color is never updated.
if (selectedView != null) {
selectedView.setTextColor(inactiveTextColor);
}
selectedView = (TextView) getChildAt(position);
selectedView.setTextColor(activeTextColor);
selectedView is held over from previous iteration of the event handler and it was noted that a good lag is experienced. This is due to the event being fired after all screen updates are settled.

So I am trying to determine best way to fix this. I can put the above code directly on the onPageSelected and it appears to work fine (limited testing) and the lag is not present on the tab change. Other method is to instill a change to the view observer tree causing the event to fire.

Comment 9

[Brad Arant]

Attachment: Bug 1565488 - Wrong panel from welcome screen is highlighted after clearing data;r?VladBaicu

Comment 10

[Brad Arant]

I went ahead and submitted the differential to move the tab text color change to the onPageSelected method and no longer wait for the global event which sometimes does not occur. I tested as many tab screens as I could find and all appears to work well and the delay noticed in the recreation of steps no longer occurs.

Comment 11

[Petru-Mugurel Lingurar [:petru]]

That part of the code where you're made changes in your patch is already problematic with lots of crashes
https://crash-stats.mozilla.org/signature/?product=FennecAndroid&signature=java.lang.NullPointerException%3A%20at%20org.mozilla.gecko.home.TabMenuStripLayout%241.onGlobalLayout%28TabMenuStripLayout.java%29&date=%3E%3D2019-07-22T13%3A48%3A00.000Z&date=%3C2019-07-29T13%3A48%3A00.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_columns=install_time&_columns=startup_crash&_columns=android_device&_columns=android_version&_sort=-date&page=1
By moving that problematic code above the guards we already have I'm afraid to not expose ourselves to even more of such issues.
Can you please make sure following this STRs https://bugzilla.mozilla.org/show_bug.cgi?id=1562622#c13 with your patch do not result in crashes?

Comment 12

[Eliza Balazs]

Hi!
There is a similar behavior on ESR 68.1b5, Beta 69.0b11, Nightly 70.0a1 (2019-08-07) with OnePlus 5T for the Top Sites and History panels from Awesomescreen:

1. Launch Fennec and go to about:home;
2. Tap on the "History" panel;
3. Tap on the "Top Sites" panel.

Expected result:
The "Top Sites" panel is highlighted.

Actual result:
The "Top Sites" panel is not highlighted.

Notes:

• Not reproducible when taping from "History" panel to the other panels.
• Not reproducible on RC 68.0.

Comment 13

[Brad Arant]

Ah, I see this as well. I believe I might know what is happening in there. I will continue with a new differential under this bug report if that works for you.

Comment 14

[Brad Arant]

Please verify that you have added patch successfully. The result of the testing I believe is without the patch applied and I was able to recreate until I applied this patch, which fixed the issue. I can resubmit patch if there is a conflict with another issue related to this area of code mentioned previously by Vlad in comment 11. That issue was fixed as well and affects a line of code immediately which may have confused the patch application.

Comment 15

[Petru-Mugurel Lingurar [:petru]]

There wasn't an uplift request to ESR so indeed this patch would not be available to test.
Brad, please file for a Beta/ESR68 approval when you get the chance.

Comment 16

[Brad Arant]

Comment on attachment 9081474 [details]
Bug 1565488 - Wrong panel from welcome screen is highlighted after clearing data;r?VladBaicu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Requested
• User impact if declined: Bug will remain unresolved.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): No new code but a reloction of existing function to correct position.
• String or UUID changes made by this patch:

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9028adf52037
Wrong panel from welcome screen is highlighted after clearing data;r=VladBaicu

Comment 18

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/9028adf52037

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9081474 [details]
Bug 1565488 - Wrong panel from welcome screen is highlighted after clearing data;r?VladBaicu

Approved for Fennec 68.1b7.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6b61417a7944

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/cdf3353038f2

Comment 22

[Laurentiu Apahidean]

I have tested the issue on Nightly 70.0a1 (2019-08-14) using a Samsung Galaxy S9 (Android 8.0.0) and the issue no longer occurs. I will mark this issue as verified in Firefox 70.

Comment 23

[Andrei Jolțan [:Andi]]

Hi, verified as fixed on Firefox 68.1b7 using Samsung Galaxy S9 (Android 8.0.0) and a Samsung Galaxy S7 (Android 7)

Comment 24

[Laurentiu Apahidean]

I have tested the issue on Beta 69.0b16 using a Pixel 3a XL (Android 9) and the issue no longer occurs. I will mark this issue as verified in Firefox 69.