brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.75k stars 2.32k forks source link

Location-permissions icon not dynamically removed from URL bar on permissions-lifetime timeout. #15473

Open stephendonner opened 3 years ago

stephendonner commented 3 years ago

Description

Location-permissions icon not dynamically removed from URL bar on permissions-lifetime timeout.

Found while testing https://github.com/brave/brave-browser/issues/14126 - so am not sure this is a bug, but wanted to cover it.

Steps to Reproduce

  1. launch Brave using --enable-logging=stderr --vmodule="*/bat-native-ledger/*"=6,"*/brave_rewards/*"=6,"*/bat-native-ads/*"=6,"*/bat-native-confirmations/*"=6,"*/brave_ads/*"=9,"*/brave_user_model/*"=6 --permission-lifetime-test-seconds=20
  2. via brave://flags, set the Permissions Lifetime flag to Enabled
  3. restart Brave again using the above command-line parameters
  4. load https://permission.site
  5. click on Location
  6. choose 20 seconds from the Give permission prompt
  7. click Allow
  8. click on the location-permission icon in the URL bar
  9. confirm it reads Continue allowing this site to access your location
  10. wait the remainder of the 20 seconds
  11. click again on the location-permission icon in the URL bar

Actual result:

The location-permissions icon:

Screen Shot 2021-04-23 at 11 21 18 AM

But brave://settings/content/location returns to defaults, with Ask before accessing (recommended) toggled on.

Screen Shot 2021-04-23 at 11 21 27 AM

Expected result:

Not 100% sure; should the location-permission icon dynamically be removed once the permissions-lifetime duration expires?

If I reload the page, it's gone, which matches the restored defaults I find in brave://settings/content

Screen Shot 2021-04-23 at 11 22 21 AM

Reproduces how often:

100%

Brave version (brave://version info)

Brave 1.24.72 Chromium: 90.0.4430.72 (Official Build) dev (x86_64)
Revision b6172ef8d07ef486489a4b11b66b2eaeed50d132-refs/branch-heads/4430@{#1233}
OS macOS Version 11.2.3 (Build 20D91)

@goodov @pes10k what's the correct behavior, here? Thanks in advance!

goodov commented 3 years ago

I think it would be nice to clean up this, but not sure if it's worth it. Given that this only happens when the page is opened and it got hit with a 24hrs/1week trigger (short time periods are available only while we test it currently). Maybe if we'll add short time periods (like 1hr), this can be elevated.