nvaccess / nvda

NVDA, the free and open source Screen Reader for Microsoft Windows
https://www.nvaccess.org/
Other
2.11k stars 636 forks source link

NVDA fails to uninstall add-on which contains read-only files #12824

Open lukaszgo1 opened 3 years ago

lukaszgo1 commented 3 years ago

Discovered when working on #12792

Steps to reproduce:

  1. Install an add-on containing read-only files (in my case this was because I've been using git to version the content of the plugin and git marks some of its hidden files as read-only).
  2. Try to uninstall the plugin.

    Actual behavior:

    NVDA fails to finish uninstallation and asks user to remove the folder manually.

    Expected behavior:

    Plugin should uninstall normally since even though it contains read-only files Windows Explorer can remove its folder successfully.

    System configuration

    NVDA installed/portable/running from source:

    Tested from sources but probably not relevant

    NVDA version:

    Latest master as of September 9th.

    Windows version:

    N/A

    Name and version of other software in use when reproducing the issue:

    None

    Other information about your system:

    None relevant

    Other questions

    Does the issue still occur after restarting your computer?

    Yes

    Have you tried any other versions of NVDA? If so, please report their behaviors.

    No, but this behavior is not new

    If add-ons are disabled, is your problem still occurring?

    No but this is about NVDA's interaction with add-ons

    Does the issue still occur after you run the COM Registration Fixing Tool in NVDA's tools menu?

    Not tried since it is not relevant.

Technical:

The problem occurs because we're using shutil.rmtree to remove the add-on directory and this function is incapable of removing read-only files. The only solution is to use different way of removing the folder. My first thought was to use SHFileOperationW we even have necessary structures defined in shellapi. Unfortunately while this solution works from the standard Python interpreter it cannot be used from NVDA currently. After a lot of research it turns out that Python 3.5 up to 3.7 which we're using has a bug which prevents usages of null terminated strings in ctypes structures and SHFileOperationW uses null as a path separator. I see the following ways of dealing with this problem:

Adriani90 commented 1 year ago

cc: @seanbudd

seanbudd commented 1 year ago

Given this is blocked by a python upgrade I am adding this to the 2024.1 milestone

seanbudd commented 11 months ago

Now that we are on Python 3.11 we plan to take this on, unless you plan to @lukaszgo1

lukaszgo1 commented 11 months ago

@seanbudd From my initial comment, this is (or was) blocked by the Python 3.11 upgrade if and only if, we prefer to use SHFileOperationW, which is legacy, and no longer recommended by Microsoft for applications which do not need to target Windows XP. I can work on this and try to use IFileOperation which is the more modern replacement, though I haven't worked with COM that much, so it might take a while. On the other hand I don't think there is a particular rush, as this issue is encountered pretty rarely.