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

Request Off-The-Record mode #28750

Closed
pilgrim-brave opened this issue Feb 24, 2023 · 7 comments · Fixed by brave/brave-core#17365
Closed

Request Off-The-Record mode #28750

pilgrim-brave opened this issue Feb 24, 2023 · 7 comments · Fixed by brave/brave-core#17365

Comments

@pilgrim-brave
Copy link

Design spec: https://docs.google.com/document/d/1J1x-pNyT5CENzdb-RxzPxLxFLE4c_PRlz6T-usJUQR4

@stephendonner
Copy link
Collaborator

Verification PASSED using

Brave 1.53.74 Chromium: 114.0.5735.53 (Official Build) beta (x86_64)
Revision c499d7ea22c8b2dba278465a5df7b86a8efa4e64-refs/branch-heads/5735@{#970}
OS macOS Version 11.7.7 (Build 20G1345)

Steps:

  1. installed 1.53.74
  2. set brave://flags/#brave-request-otr-tab to Enabled
  3. visited a request-OTR enrolled site: loveisrespect.org
  4. confirmed that the “would you like OTR mode” interstitial appears
  5. clicked Proceed Off-The-Record to go into OTR mode
  6. confirmed that the requested page appears, and that there is an info bar at the top of the page describing how to leave OTR mode
  7. opened dev tools went into the console and ran this code: window.localStorage.braveQATest = true
  8. closed the OTR’ed site
  9. confirmed that the OTR’ed site was not included in your browsing history
  10. revisited loveisrespect.org, this time declining OTR mode
  11. opened the dev tools console and ran this in the console: console.log(window.localStorage.braveQATest)

Confirmed that undefined was returned

Step 2 Step 5 Step 6 Step 6.5 Step 7 Step 10 Step 11
Screen Shot 2023-05-30 at 5 35 23 PM Screen Shot 2023-05-30 at 5 35 19 PM Screen Shot 2023-05-30 at 5 36 21 PM Screen Shot 2023-05-30 at 5 36 25 PM Screen Shot 2023-05-30 at 5 37 13 PM Screen Shot 2023-05-30 at 5 37 55 PM Screen Shot 2023-05-30 at 5 38 46 PM
brave://history, step 9 brave://history, step 10+
Screen Shot 2023-05-30 at 5 51 34 PM Screen Shot 2023-05-30 at 5 51 27 PM

@kjozwiak
Copy link
Member

kjozwiak commented Jun 1, 2023

This will also eventually enabled via Griffin as per brave/brave-variations#640 & brave/brave-variations#642. Assuming we'll also eventually enable on 1.53.x once the feature has been enabled on Nightly. Just a heads for @brave/qa-team. Verifying via brave://flags works for now but we'll want to double check that it's working via Griffin on release before 1.53.x goes live.

@MadhaviSeelam MadhaviSeelam added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jun 2, 2023
@MadhaviSeelam
Copy link
Collaborator

Verification PASSED using

Brave | 1.53.76 Chromium: 114.0.5735.90 (Official Build) beta (64-bit)
-- | --
Revision | 386bc09e8f4f2e025eddae123f36f6263096ae49-refs/branch-heads/5735@{#1052}
OS | Windows 11 Version 22H2 (Build 22621.1702)

Case 1: Proceed to OTR - PASSED

  1. installed 1.53.76
  2. launch Brave
  3. set brave://flags/#brave-request-otr-tab to Enabled
  4. visited a request-OTR enrolled site: https://navegandolibres.org/
  5. confirmed that the “would you like OTR mode” interstitial appears
  6. clicked Proceed Off-The-Record to go into OTR mode
  7. confirmed that the requested page appears, and that there is an info bar at the top of the page describing how to leave OTR mode
  8. opened dev tools went into the console and ran this code: window.localStorage.braveQATest = true
  9. closed the OTR’ed site
  10. confirmed that the OTR’ed site was not included in your browsing history
step 3 step 5 step 7 step 8 step 10
image image image image image

Case 2: Decline OTR mode - PASSED

  1. Continue from Case 1
    • closed the OTR’ed site
    • no record of OTR'ed site in brave://history`
  2. visited https://navegandolibres.org/ in a new tab,
  3. clicked Proceed Normally to decline OTR mode
  4. verified info bar at the top of the page is not available
  5. opened the dev tools console and ran this in the console: console.log(window.localStorage.braveQATest)

Confirmed that undefined was returned
Confirmed brave://history shows the OTR’ed site

ex ex ex ex ex
image image image image image

Case 3: Turn Off from the infobar - PASSED

1.new profile
2.launch Brave
3. set brave://flags/#brave-request-otr-tab to Enabled
4. visited a request-OTR enrolled site: https://navegandolibres.org/
5. clicked Proceed Off-The-Record to go into OTR mode
6. clicked Turn off in the infobar
7. opened brave://history

Confirmed brave://history shows the OTR’ed site

step 4 step 6 step 7
image image image

@MadhaviSeelam MadhaviSeelam added QA Pass-Win64 and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Jun 3, 2023
@MadhaviSeelam MadhaviSeelam added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Jun 10, 2023
@MadhaviSeelam
Copy link
Collaborator

MadhaviSeelam commented Jun 10, 2023

Verification PASSED using

Brave | 1.53.85 Chromium: 114.0.5735.110 (Official Build) beta (64-bit)
-- | --
Revision | 1c828682b85bbc70230a48f5e345489ec447373e-refs/branch-heads/5735_90@{#13}
OS | Linux

Case 1: Proceed to OTR - PASSED

  1. installed 1.53.85
  2. launch Brave
  3. set brave://flags/#brave-request-otr-tab to Enabled
  4. visited a request-OTR enrolled site: https://www.ceta.tech.cornell.edu/
  5. confirmed that the “would you like OTR mode” interstitial appears
  6. clicked Proceed Off-The-Record to go into OTR mode
  7. confirmed that the requested page appears, and that there is an info bar at the top of the page describing how to leave OTR mode
  8. opened dev tools went into the console and ran this code: window.localStorage.braveQATest = true
  9. closed the OTR’ed site
  10. confirmed that the OTR’ed site was not included in your browsing history
step 3 step 5 step 7 step 8 step 10
image image image image image

Case 2: Decline OTR mode - PASSED

  1. Continue from Case 1
    • closed the OTR’ed site
    • no record of OTR'ed site in brave://history`
  2. visited https://www.ceta.tech.cornell.edu/ in a new tab,
  3. clicked Proceed Normally to decline OTR mode
  4. verified info bar at the top of the page is not available
  5. opened the dev tools console and ran this in the console: console.log(window.localStorage.braveQATest)

Confirmed that undefined was returned
Confirmed brave://history shows the OTR’ed site

ex ex ex ex
image image image image

Case 3: Turn Off from the infobar - PASSED

1.new profile
2.launch Brave
3. set brave://flags/#brave-request-otr-tab to Enabled
4. visited a request-OTR enrolled site: https://www.ceta.tech.cornell.edu/
5. clicked Proceed Off-The-Record to go into OTR mode
6. clicked Turn off in the infobar
7. verified infobar is dismissed
8. opened brave://history

Confirmed brave://history shows the OTR’ed site

step 3 step 4 step 6 step 7 step 8
image image image image image

@sangwoo108
Copy link

sangwoo108 commented Aug 22, 2023

Hi, everyone. I'm a bit worried about OTR tab. Many UI components in a browser window are assuming that every tabs and BrowserKeyedService is using same Profile(BrowserContext) with that of Browser object. So it's quite common practice for UI code to get profile from the Browser object. Not 100% sure but I kind of feel like maybe we're exposing potential risk.

In the previous patch, it looks like we've done

  • Reload web contents
  • Override history service
  • Add Network throttle
    right? Sometimes we get browser context keyed services from WebContetns's BrowserContext, so I'm wondering if we've switched browser context for the web contents too. Otherwise, we might access the wrong service or profile for the contents.

I think opening a new incognito window from the interstitial page would be much safer.

cc @petemill @simonhong @bsclifton

@pes10k
Copy link
Contributor

pes10k commented Aug 23, 2023

Opening a new incognito window doesn’t solve the problem we’re aiming to solve here, for reasons explained in the spec.

However, @pilgrim-brave and @bridiver have a new approach for OTR-tabs that we think will more comprehensively address these concerns. Though one of them could explain the how and why better than me

@sangwoo108
Copy link

Thanks @pes10k for the explanation. I agree that OTR tab is worthy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment