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

Add-on store: Improve disk management for launcher and secure copies #14955

Merged
merged 2 commits into from Jun 2, 2023

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Jun 1, 2023

Link to issue number:

Part of #13985
Originally raised by @XLTechie in #14912 (reply in thread)

Summary of the issue:

The installer for add-on store versions of NVDA, is creating the addonStore folder in %appdata%\nvda, even before the license agreement is accepted!
E.g. If starting the installer, and selecting "exit" instead of agreeing to the license agreement, the addonStore directory is created and populated with some files and directories under %appdata%\nvda.
It won't create %appdata%\nvda, but if that already exists, it will create the store directory.

This is absolutely not expected behavior. NVDA should never be doing any directory and file creation on the local system before "install" is selected.
I have not looked at the code, so do not know what this would do to an existing installed store copy, but really no user would expect that the installed NVDA's configuration could be altered by an installer that was not told to "install".

Description of user facing changes

Ensure the caching behaviour of add-ons matches that of NVDA's config.
Files should only be written to disk when not running as launcher nor in a secure context.

Description of development approach

Adds relevant checks to prevent saving the add-on handler state and add-on store caches to disk.

Testing strategy:

  • Manual testing to be confirmed

Known issues with pull request:

None

Change log entries:

N/A

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@seanbudd seanbudd changed the base branch from master to addonStore-base June 1, 2023 06:34
@AppVeyorBot
Copy link

See test results for failed build of commit f87035a14f

@XLTechie
Copy link
Contributor

XLTechie commented Jun 1, 2023 via email

@seanbudd seanbudd changed the base branch from addonStore-base to master June 1, 2023 23:58
@seanbudd seanbudd changed the base branch from master to addonStore-base June 1, 2023 23:58
@seanbudd seanbudd marked this pull request as ready for review June 2, 2023 04:46
@seanbudd seanbudd requested a review from a team as a code owner June 2, 2023 04:46
@seanbudd seanbudd requested review from michaelDCurran and removed request for a team June 2, 2023 04:46
@seanbudd seanbudd merged commit 60581ac into addonStore-base Jun 2, 2023
1 check passed
@seanbudd seanbudd deleted the addonStore-cacheOnlyInstall branch June 2, 2023 05:19
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Jun 2, 2023
seanbudd added a commit that referenced this pull request Jun 6, 2023
Fix up of #14955
Raised in #14912 (comment)

Summary of the issue:
Caching directories are still created when NVDA should not write to disk

Description of user facing changes
Caching directories are no longer created when NVDA should not write to disk

Description of development approach
Caching directories are no longer created when NVDA should not write to disk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants