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

Keep the menu items in private windows with Tor same as in others. #681

Closed
wants to merge 1 commit into from

Conversation

riastradh-brave
Copy link
Contributor

fix brave/brave-browser#1627

The result is a little silly -- because private windows with Tor are
implemented in a guest session, all new windows open in the same
session, whether with 'New window' or 'New incognito window' (soon to
be 'New private window') or 'New private window with Tor').

On the other hand, this is perhaps justifiable because we don't want
to make it easy to accidentally stop using Tor. You can do that
explicitly through the profile menu -- but not just by casually
hitting Ctrl-N.

On the third hand, control-click on links greys out the 'Open link
in private window' item when you're in a private window, rather than
making it just...do what it says. ('Open link in new window' also
opens in a new private window in that case.) For consistency, maybe
we should do that instead, when we are on less of a deadline?

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Verified that these changes build without errors on
    • Windows
    • macOS
    • Linux
  • Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests) on
    • Windows
    • macOS
    • Linux
  • Ran git rebase master (if needed).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.
  • Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

fix brave/brave-browser#1627

The result is a little silly -- because private windows with Tor are
implemented in a guest session, all new windows open in the same
session, whether with 'New window' or 'New incognito window' (soon to
be 'New private window') or 'New private window with Tor').

On the other hand, this is perhaps justifiable because we don't want
to make it easy to accidentally _stop_ using Tor.  You can do that
explicitly through the profile menu -- but not just by casually
hitting Ctrl-N.

On the third hand, control-click on links _greys out_ the 'Open link
in private window' item when you're in a private window, rather than
making it just...do what it says.  ('Open link in new window' also
opens in a new private window in that case.)  For consistency, maybe
we should do that instead, when we are on less of a deadline?
Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

screen shot 2018-11-07 at 18 19 09

`New Window` and `New Incognito Window` opens same type of window in tor window because in tor window we can only open tor window. `New Private Window with Tor` does nothing because we are already in tor profile.

I think we need to wait for tor session window to make each items working properly.
If this is really urgent, there should be only one menu item called New Private Window with Tor in tor window with corresponding keyboard shortcut

@bbondy
Copy link
Member

bbondy commented Nov 20, 2018

Haven't seen discussion on this in a couple weeks and I think we have to wait for non-guest window Tor profiles for this to work properly. So closing the PR for now.

@bbondy bbondy closed this Nov 20, 2018
@bsclifton bsclifton deleted the riastradh-1627-tormenufixes branch November 8, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Menu fixes for private windows with Tor
3 participants