search

brave / browser-laptop

[DEPRECATED] Please see https://github.com/brave/brave-browser for the current version of Brave
https://www.brave.com
Other
7.95k stars 977 forks source link

Deprecation and Obsoletion experience #15349

Closed petemill closed 5 years ago

petemill commented 5 years ago

Submitter Checklist:

Test Plan:

1

From fresh profile or any profile which has not run with this code yet:

2

Using profile from 1), either adjust clock forward 10 days, or deprecatedOn time in profile state file back at least 10 days

3

There is no option to update the browser, nor is there any message saying that there are no new updates to the browser.

Reviewer Checklist:

Tests

petemill commented 5 years ago

Added unit tests because time and math...

petemill commented 5 years ago

Un-marking as WIP now since all features are now implemented. I still want to take another look at the code tomorrow as well as add test plans here and possibly a couple more unit tests and a way to manually test.

image

image

bsclifton commented 5 years ago

Fresh profile - I'm getting the fireworks ☹️ Which means I can't use the browser (during the 10 day period). Also, check for update happens (at least when running from source) and fails, showing a 2nd bar

Screen Shot 2019-04-04 at 1 08 34 PM
diracdeltas commented 5 years ago

i doubt anyone will bother to do this, but just noting that someone can avoid Brave getting into the unusable state by setting their clock back

petemill commented 5 years ago

Yeah @diracdeltas there are a couple ways to get around it (e.g. adjust the state file to put a deprecation date in the future, open the console and change window.location, build from source...), but the aim is to make it extremely obvious and pressured, not necessarily to be 100% secure AFAIK. If anyone has a different opinion then lmk.

petemill commented 5 years ago

Test Plan added. I'm done unless I get any further feedback @bsclifton

bsclifton commented 5 years ago

Couple of quick notes before reviewing 😄

I did the following:

We'll want to make this version 0.27.x. I'll review what we have here and then create a commit with that version

bsclifton commented 5 years ago

macOS and Linux builds available here: https://github.com/brave/browser-laptop/releases

Didn't get a chance to fully test/review yet- also need to sort out issues with signing on Windows build servers

srirambv commented 5 years ago

GitHub shows insecure icon,when opening the rev version from about:brave image However opening support page from Learn more... shows secure connection for support.brave.com image

srirambv commented 5 years ago

Context menu should probably be removed once the 10 days limit is reached. You can still continue to browse using the search functionality. Two options

  1. Disable context menu
  2. Remove Search option in context menu

I'd vote for option1 which makes it more hard to use the browser and forces to switch to bc muon

srirambv commented 5 years ago

Disable payment optin message if user has not opted into payments image

Edit: Remove/hide Payments/Sync options in about:preferences if user has not opted in payments or created a sync chain

srirambv commented 5 years ago

Setting system to an older date still give the browser to show X days left deprecation message and make it fully useable. Once the 10 days are completed irrespective of user manually setting system date to an older date, it should still show that browser has completed the 10 days grace period of usage from upgrade to 0.27.0 and make it useless.

bsclifton commented 5 years ago

Pushed some updates:

RE: the context menu- that is a good catch. But I think we're fine not fixing it. It's an edge case and would be a painfully inefficient way to use the browser in a daily driver context

petemill commented 5 years ago

Setting system to an older date still give the browser to show X days left deprecation message and make it fully useable. Once the 10 days are completed irrespective of user manually setting system date to an older date, it should still show that browser has completed the 10 days grace period of usage from upgrade to 0.27.0 and make it useless.

We’re not trying to make it securely impossible to browse any pages. Rather we are just disabling the obvious UX. After all, if someone really wanted to keep using this version, they could run from source, update the state JSON file with a different deprecatedOn date, or download an older release and disable connection to update server. There’s a line but I think we decided that the obvious functionality to disable is the address bar, tab restore on startup, and links from other apps. The other side of that line would be usage scenarios that are just too painful for someone to keep using this version as their main browser, like running with a clock that’s always in the past.

GeetaSarvadnya commented 5 years ago

After 10 days user can still access Bookmarks, should it be allowed?

image

User can still pin the websites into Payments which are bookmarked.

image

petemill commented 5 years ago

Just pushed functionality which blocks bookmarks bar items from opening content, when obsolete. Stopped short of blocking context menu on bookmarks page as I did not want to add more risk to normal website operations for downloading the latest version.

GeetaSarvadnya commented 5 years ago

Observations on 0.27.2

  1. Toolbar bookmarks are not accessible/cannot be opened in NTP – Looks good
  2. From the toolbar, imported bookmark sites cannot be opened in NTP – Looks good
  3. Payments is still accessible (User can still back up their wallet) – Looks good
  4. Existing bookmarks can be exported to .HTML (User can take back up of BM’s) – Looks good
  5. Bookmarks are accessible and which can be opened in NTP via context menu through Bookmarks>Bookmarks Manager>Bookmarks Toolbar -??
  6. Imported bookmarks are NOT accessible via Bookmarks manager – Looks good
  7. Sites added in a-c table can be opened in NTP via Context menu -??
  8. No option to Update muon – Looks good
  9. NTP always displays obsoletion UI and prompts to either i) launch or ii) install brave-core – Looks good
  10. Tabs and windows are no longer restored – Looks good
  11. Address bar is not useable – Looks good

After 10 days user can still access the sites and links ( step 5 and 7). Is that ok? should we allow this?

bsclifton commented 5 years ago

@GeetaSarvadnya yup- that is OK, IMO 😄Eventually sites in AC will drop off and that is a pretty painful way to experience the internet

The context menu is something @srirambv noted too - I think it's OK, as browsing via context menu is definitely a bad experience

Thanks for testing so quickly and great job being so thorough! 😄

LaurenWags commented 5 years ago

Mentioned in channel, but will document here as well: 2.c) NTP always displays obsoletion UI and prompts to either 2.c.i)launch or 2.c.ii) install brave-core If user has 'A new tab shows' set to something other than 'Dashboard', the fireworks/obsoletion UI will not display after 10 days. We should probably force new tab page to show dashboard with warning message prior to 10 days.

LaurenWags commented 5 years ago

mentioned in channel, will document here as well: In the following scenario I'm not getting the "fireworks" NTP. Here are my steps:

  1. Install 0.25.304, make it your default browser --> update to 0.27.2 (keep NTP as Dashboard)
  2. Visit sites, etc as normal. Ignore warnings.
  3. Close Brave, set computer clock ahead 10 days.
  4. Click on a link from an app (such as Slack)
  5. Muon launches and shows the link you clicked on. Note, if you open muon, then click on a link, the link does not open - you see fireworks page (this is expected)
GeetaSarvadnya commented 5 years ago

Verified both scenarios which @LaurenWags mentioned on version 0.27.3 both scenarios working fine on windows

Scenario1: https://github.com/brave/browser-laptop/pull/15349#issuecomment-481860953

Scenario2: https://github.com/brave/browser-laptop/pull/15349#issuecomment-481884322

LaurenWags commented 5 years ago

0.27.3 looks good on macOS

btlechowski commented 5 years ago

0.27.3 looks good on Windows 10 x64

Minor issues found:

kjozwiak commented 5 years ago

Verification PASSED on macOS 10.14.4 x64 using the following build:

Brave: 0.27.3 
rev: 2df8cb101008dce51a22fe652dcf22b00c21f62e 
Muon: 8.1.8 
libchromiumcontent: 69.0.3497.100

Went through the following cases:

kjozwiak commented 5 years ago

@bsclifton @rebron looks like we're in good shape for the Monday release:

However, as mentioned under Slack, there's no way we'll be able to upgrade Linux as the repo and snaps have been converted to b-c.

@GeetaSarvadnya @LaurenWags @btlechowski @srirambv thanks for your all hard work and thoroughness!