Opened 6 years ago

Closed 5 years ago

#8400 closed defect (fixed)

Torbutton's browsing history pref seems to require restart

Reported by: mikeperry Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-rebase-regression, tbb-usability, tbb-torbutton, MikePerry201502, TorBrowserTeam201502
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This is probably because we now rely on the browser.privatebrowsing.autostart, which I guess doesn't actually change the private browsing state itself. We probably need to emit the private browsing change notification event, or otherwise request changing the private browsing state the official way.

Child Tickets

Attachments (1)

pb-autostart-prompt.png (41.8 KB) - added by mcs 6 years ago.
Firefox 24 restart prompt

Download all attachments as: .zip

Change History (9)

comment:1 Changed 6 years ago by mikeperry

We should probably also listen to these observers and fully sync our pref with private browsing mode: https://developer.mozilla.org/en-US/docs/Supporting_private_browsing_mode

comment:2 Changed 6 years ago by mikeperry

Keywords: tbb-usability added

Changed 6 years ago by mcs

Attachment: pb-autostart-prompt.png added

Firefox 24 restart prompt

comment:3 Changed 6 years ago by mcs

Kathy Brade and I did a little research on this bug. In Firefox 20 and newer, a prompt that reads "You must restart Firefox to enable this feature" is displayed when someone tries to toggle the browser.privatebrowsing.autostart pref. via the Options window. It seems like the Torbutton pref. UI should also prompt to restart. Existing browser windows will need to be closed to switch to private browsing mode anyway, and it is probably rare for users to toggle this pref. There is more cost to a restart with TBB of course due to the need to reestablish a Tor connection.

Should we add such a prompt (and associated restart logic)?

comment:4 Changed 5 years ago by erinn

Component: TorBrowserButtonTor Browser
Keywords: tbb-torbutton added
Owner: changed from mikeperry to tbb-team

comment:5 Changed 5 years ago by mikeperry

Keywords: MikePerry201502 TorBrowserTeam201502 added

comment:6 Changed 5 years ago by mikeperry

Keywords: TorBrowserTeam201502R added; TorBrowserTeam201502 removed
Status: newneeds_review

Ok, this should be fixed in my 1.8-next branch. Here's the commit: https://gitweb.torproject.org/mikeperry/torbutton.git/commit/?h=1.8-next&id=b4540c02a45d9f2c7153c72cee7b2d8f1fa12591

Please review?

comment:7 Changed 5 years ago by brade

Keywords: TorBrowserTeam201502 added; TorBrowserTeam201502R removed
Status: needs_reviewneeds_revision

Mark and I reviewed your fix. It looks good. Just a few minor comments:

  • There is an extra blank line after the definition of Cc and Ci.
  • Please use the Cc and Ci constants in the "var sb = ..." statement.
  • You could replace more occurrences of "var" with "let".
  • Remove the extra '.' at the end of your commit message.

comment:8 Changed 5 years ago by mikeperry

Resolution: fixed
Status: needs_revisionclosed

Ok, I made the suggested updates to the code, but I left the .. at the end of the commit message in haste. Juggling too many things, I guess. Thanks for the review!

Note: See TracTickets for help on using tickets.