Ticket #297 (closed defect: fixed)

Opened 5 years ago

Last modified 4 years ago

Settings Error icon behaviour

Reported by: dmurgy Owned by:
Priority: minor Milestone: Eraser 6.0.8
Component: User Interface (UX) Version: 6.0.6.1376
Keywords: Cc:
Processor Architecture: Blocked By:
Blocking: Operating System:

Description

If you have listed some files in the "...plausible deniability" settings that you then delete are re-shown when eraser is restarted.

Steps to reproduce


  1. Add files to "...plausible deniability" list.
  2. Save Settings
  3. Close Eraser
  4. re-start Eraser.
  5. Select and remove all files in "...plausible deniability" area
  6. Save Settings
  7. Close Eraser
  8. re-start Eraser.

If will now see that the files reappear.

Also when you have removed all files you get the "flashing" warning information icon, but when you add a new file to this list the icon is still there saying that they are no files even when you have listed new ones. It only goes away when you have hit the "Save settings" button.

Blocking

IdSummaryMilestone
#297Settings Error icon behaviourEraser 6.0.8

Blocked by

IdSummaryMilestone
#297Settings Error icon behaviourEraser 6.0.8

Attachments

eraser - icon on plausible deniability.jpg (63.7 KB) - added by dmurgy 5 years ago.
icon issue
eraser - plausible deniability files list issue.jpg (78.5 KB) - added by dmurgy 5 years ago.
list files issue
ticket-297-SettingsPanel-patch.txt (547 bytes) - added by dmurgy 5 years ago.

Change History

Changed 5 years ago by dmurgy

icon issue

Changed 5 years ago by dmurgy

list files issue

comment:1 Changed 5 years ago by Joel

  • Milestone set to Eraser 6.0.7

comment:2 Changed 5 years ago by Joel

  • Status changed from new to pending

Well, you're seeing this because when the Save Settings call failed with the error icon the settings will not be saved, since the settings were not saved, the old files remain on the list.

I don't see how multiple instances of the same file will affect the behaviour, but it definitely will cause the file with the highest number of occurrences to be picked more often as a decoy file.

As for the icon being permanent until Save Settings is clicked, that's by design since Saving the settings will be committing the changes to the system. Unless you have an alternative as to how the error icon be shown?

The alternative I am aware of will be to validate each control on the page (using the validating and validate events) and showing the error icon when the validating event is called and removing it when the validate event is triggered. Would this be a better idea?

comment:3 Changed 5 years ago by dmurgy

  • Status changed from pending to new

So there's a bug in the "Save Settings" button then didn't clear out the entries?

Perhaps some logic needs to be added so that a check is done to see if the filename already exists in the list and if so then generate error.

You can add some extra code on the "Add file", "Add Folder" and "Remove" buttons so that at the end a check is done to see how many files exist in the list (UI control, not in the registry). If none then display the error icon, otherwise hide it. As far as I can see then this is the only control with an error icon being display on the "settings" page.

comment:4 Changed 5 years ago by dmurgy

  • Status changed from new to pending

Here is an updated plausableDeniabilityFilesRemoveUpdate() function from SettingsPanels?.cs.

Changed 5 years ago by dmurgy

comment:5 Changed 5 years ago by dmurgy

  • Status changed from pending to new

Attachment (ticket-297-SettingsPanel?-patch.txt) added by ticket reporter.

comment:7 Changed 5 years ago by Joel

  • Status changed from new to pending

Oh, I found the problem -- see line 432 of settingspanel.cs:

            IList<string> plausibleDeniabilityFilesList =
                managerSettings.PlausibleDeniabilityFiles;
            foreach (string str in this.plausibleDeniabilityFiles.Items)
                plausibleDeniabilityFilesList.Add(str);

before the foreach there should be a plausibleDeniabilityFilesList.Clear(); call so that deleted items are deleted. See r1517.

Your patch would move the error icon code from the Save button to the plausible deniability enable/disable handler. Wouldn't this cause the error handling for the plausible deniability code to be separate from the rest of the handlers? More error conditions can be triggered: when the Default PRNGs and Erasure methods are missing/not loaded, when an invalid UI language is selected (try setting in the registry)

comment:8 Changed 5 years ago by dmurgy

  • Status changed from pending to new

My patch has copied the error handling from the save button function that's all. Different people code different ways, some put checking on functions whilst others put it all in 1x place. I prefer to put checking on the functions.

Another way to do this is to have a separate function that handles all of the error handling and you call this from what ever other function you wish to perform validation from. This function would have 1x input parameter which would be either the specific validation to do or a global "all" to check everything, such as:

if (!errorCheck("all"))
retturn;

bool errorCheck(String Section)
{

if (Section == "All"
Section == "plausible")

{

perform error check...

}
return true;

}

comment:9 Changed 5 years ago by Joel

  • Component changed from Core to User Interface (UX)

comment:10 Changed 5 years ago by Joel

  • Summary changed from Files / folders listed in "....plausible deniablility" area are reshown after deleting when re-started eraser to Warning icon behaviour

comment:11 Changed 5 years ago by Joel

  • Summary changed from Warning icon behaviour to Settings Error icon behaviour

comment:12 Changed 4 years ago by Joel

  • Status changed from new to closed
  • Resolution set to invalid

The initial problem about files not being removed has been fixed; as to the behaviour change for error checking, I've reconsidered it and still think the current behaviour is most appropriate.

comment:13 Changed 4 years ago by Joel

  • Status changed from closed to reopened
  • Resolution invalid deleted

Hold on, it should be fixed.

comment:14 Changed 4 years ago by Joel

  • Status changed from reopened to closed
  • Resolution set to fixed
Note: See TracTickets for help on using tickets.