Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

retire Binance widget rather than completely removing #19888

Closed
kjozwiak opened this issue Dec 3, 2021 · 6 comments · Fixed by brave/brave-core#11485
Closed

retire Binance widget rather than completely removing #19888

kjozwiak opened this issue Dec 3, 2021 · 6 comments · Fixed by brave/brave-core#11485
Assignees
Labels
bug feature/widgets OS/Android Fixes related to Android browser functionality priority/P1 A very extremely bad problem. We might push a hotfix for it. QA Pass - Android ARM QA/Yes release/blocking release-notes/include

Comments

@kjozwiak
Copy link
Member

kjozwiak commented Dec 3, 2021

Description

As per discussions, we'll be reverting brave/brave-core#11148 from 1.35.x & 1.34.x and will implement the following which aligns closer to what desktop did via brave/brave-core#11266.

As per @bbondy, we'll do the following:

* Retire it to the Edit stack screen (i.e. equivalent to gallery on Desktop) for new users or if a user hasn't used it.
* Keep it there for users that have connected to it

Steps to reproduce

Test Case #1

  1. install 1.34.34 Chromium: 96.0.4664.45 which has Binance as a widget
  2. upgrade to 1.35.4 Chromium: 96.0.4664.55 and it will be completely removed (even if you're authenticated)

Actual result

Binance is being completely removed even if users are authenticated when upgrading to a version with brave/brave-core#11148.

Expected result

* Retire it to the Edit stack screen (i.e. equivalent to gallery on Desktop) for new users or if a user hasn't used it.
* Keep it there for users that have connected to it

Issue reproduces how often

100% reproducible using the STR/Cases mentioned above.

Version/Channel Information:

  • Can you reproduce this issue with the current Play Store version? N/A
  • Can you reproduce this issue with the current Play Store Beta version? Yes
  • Can you reproduce this issue with the current Play Store Nightly version? Yes

Device details

  • Install type (ARM, x86): ARM
  • Device type (Phone, Tablet, Phablet): Pixel 6
  • Android version: Android 12

Brave version

1.35.4 Chromium: 96.0.4664.55

Website problems only

  • Does the issue resolve itself when disabling Brave Shields? N/A
  • Does the issue resolve itself when disabling Brave Rewards? N/A
  • Is the issue reproducible on the latest version of Chrome? N/A

Additional information

CCing @bsclifton @rebron @srirambv @bbondy @deeppandya

@deeppandya
Copy link

@kjozwiak I would assume that the conditions would apply only for release channel as for beta and nightly we have already removed the widget. So those users would be treated as new users.

@kjozwiak
Copy link
Member Author

kjozwiak commented Dec 3, 2021

@deeppandya yup, that makes sense 👍 Just double checking though, once 1.34.x makes it into release, will we run into the above again? Assuming not as we'll revert the above code correct and as you mentioned, treat users in 1.35.x & 1.34.x as new users.

@deeppandya
Copy link

I will revert the changes on all channels but doing that may affect the users on beta and nightly as I removed the prefs which was saving the information for those users. I think we should be okay, because we haven't heard any complaints yet from the users on beta and nightly.

@bbondy
Copy link
Member

bbondy commented Dec 3, 2021

I will revert the changes on all channels but doing that may affect the users on beta and nightly as I removed the prefs which was saving the information for those users. I think we should be okay, because we haven't heard any complaints yet from the users on beta and nightly.

Sounds fine

@rebron rebron modified the milestones: 1.33.x - Release, 1.34.x - Beta Dec 7, 2021
@rebron rebron added the priority/P1 A very extremely bad problem. We might push a hotfix for it. label Dec 7, 2021
@kjozwiak
Copy link
Member Author

kjozwiak commented Jan 5, 2022

Waiting on #20331 to get addressed before we uplift brave/brave-core#11551 into 1.34.x.

@srirambv
Copy link
Contributor

srirambv commented Jan 6, 2022

Verification passed on Oppo Reno 5 with Android 11 running 1.34.80 x64 build

Clean Install - Open NTP

  1. Install 1.34.80
  2. Verified no Binance widget is shown on NTP
  3. Open a new tab still no Binance widget in the stack

Clean Install - Open Tab Manager

  1. Install 1.34.80
  2. Verified no Binance widget is shown on NTP
  3. Open tab manager and return to the same NTP no Binance widget is shown
  4. Open tab manager and open a new tab via +, no Binance widget is shown

Upgrade - Open NTP

  1. Install 1.34.106
  2. Ensure Binance is available
  3. Upgrade to 1.34.80
  4. Verified Binance is removed from the widget list
  5. Open a new tab, no Binance widget is shown

Upgrade - Tab Manager

  1. Install 1.34.106
  2. Ensure Binance is available
  3. Upgrade to 1.34.80
  4. Verified Binance is removed from the widget list
  5. Open tab tray and return to the same tab, no Binance widget is shown

Upgrade - Binance Authenticated

  1. Install 1.34.106
  2. Ensure Binance is available
  3. Authenticate Binance and move to the widget to the top of stack
  4. Upgrade to 1.34.80
  5. Verified Binance is retained in the same position before upgrade

Note: If you remove the widget on NTP and open tab tray it will still show the widget on stack but it actually isn't this is an issue with the app taking tab snapshot to show in tab tray. Same thing happens when you try to swipe tabs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug feature/widgets OS/Android Fixes related to Android browser functionality priority/P1 A very extremely bad problem. We might push a hotfix for it. QA Pass - Android ARM QA/Yes release/blocking release-notes/include
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants