Opened 8 years ago

Closed 14 months ago

#3927 closed defect (wontfix)

Vidalia torrc editor 'Save settings' checkbox is useless

Reported by: rransom Owned by: chiiph
Priority: Low Milestone:
Component: Archived/Vidalia Version: Vidalia: 0.2.20
Severity: Blocker Keywords: archived-closed-2018-07-04
Cc: onizuka.xxxx@…, cool_asis_is@…, ashishnitinpatil@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Changes made using Vidalia's torrc editor are always saved to disk, even when the 'Save settings' checkbox is unchecked.

I doubt that Vidalia can change Tor's configuration without saving the changes in torrc soon afterward, so this checkbox should be removed.

Child Tickets

Attachments (1)

TorrcDialog.diff (1.2 KB) - added by ashishnitinpatil 6 years ago.
The patch file (for both .ui & .cpp combined)

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by chiiph

Changes in torrc are applied via SETCONF control command. If the "Save settings" checkbox is check, it issues a SAVECONF control command.

So, does this happen the same if you issue a SETCONF control command by hand?

comment:2 in reply to:  1 Changed 8 years ago by rransom

Replying to chiiph:

Changes in torrc are applied via SETCONF control command. If the "Save settings" checkbox is check, it issues a SAVECONF control command.

So, does this happen the same if you issue a SETCONF control command by hand?

No. But clicking 'OK' in Vidalia's 'Settings' dialog sends SAVECONF, and nearly all users will click 'OK' there after they finish using the 'Edit current torrc' dialog.

comment:3 Changed 7 years ago by onizuka

Cc: onizuka.xxxx@… added
Version: Vidalia: 0.2.20

I agree with rransom.

A glance at the master branch's code (0.2.21) revealed that the application has not been designed to deal with a 'temporary setting' : SAVECONF is called in ServiceSettings.cpp, ConfigDialog.cpp and TorrcDialog.cpp. ConfigDialog should be the only one to handle the SAVECONF command otherwise the service editing & the torrc editing override each other - because it iterates through a stack to save the whole set of pages -, thus the undesired behaviours which occur when a SAVECONF is issued after a SETCONF.

Since the feature is not really essential and seems to have disappeared from branch alpha (0.3.3-alpha) I also suggest to remove the checkbox from the TorrcDialog.ui in the current branch master.

comment:4 Changed 6 years ago by ashishnitinpatil

I am very new to Tor Dev.

Would deleting the checkbox from TorrcDialog.ui (using Qt Creator GUI)
be both sufficient & necessary or any other changes would be required in
addition to that?

comment:5 Changed 6 years ago by ashishnitinpatil

Cc: cool_asis_is@… added

Changed 6 years ago by ashishnitinpatil

Attachment: TorrcDialog.diff added

The patch file (for both .ui & .cpp combined)

comment:6 Changed 6 years ago by ashishnitinpatil

Status: newneeds_review
  1. Deleted the checkbox "chkSave" from TorrcDialog.ui
  2. Deleted corresponding trigger "if(ui.chkSave->isChecked())" from TorrDialog.cpp

First patch from me, for Tor. Would love to make any more changes if required.

comment:7 Changed 6 years ago by ashishnitinpatil

Status: needs_reviewneeds_revision

Sorry, got the action wrong.

comment:8 Changed 6 years ago by ashishnitinpatil

Status: needs_revisionneeds_review

My bad.

comment:9 Changed 6 years ago by ashishnitinpatil

Cc: ashishnitinpatil@… added

comment:10 Changed 3 years ago by lunar

Keywords: easy removed
Severity: Blocker

comment:11 Changed 14 months ago by teor

Keywords: archived-closed-2018-07-04 added
Resolution: wontfix
Status: needs_reviewclosed

Close all tickets in archived components

Note: See TracTickets for help on using tickets.