Skip to content

Send architecture when update checking#14019

Merged
seanbudd merged 5 commits into
nvaccess:masterfrom
LeonarddeR:environment
Aug 19, 2022
Merged

Send architecture when update checking#14019
seanbudd merged 5 commits into
nvaccess:masterfrom
LeonarddeR:environment

Conversation

@LeonarddeR

@LeonarddeR LeonarddeR commented Aug 16, 2022

Copy link
Copy Markdown
Collaborator

Link to issue number:

None

Summary of the issue:

It was discovered in #12064 that NVDA sends the architecture to the update server when updating, but it doesn't properly distinguish between architectures. Therefore, NVDA should send which architecture is in use specifically.

Description of user facing changes

None, only for usage stats.

Description of development approach

Consistency of data is kept by not changing how the x64 key is determined in the dictionary.
The specific architecture is added as an extra key to the dictionary.

Testing strategy:

Known issues with pull request:

None

Change log entries:

Changes

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

Comment thread source/updateCheck.py Outdated
Comment thread source/updateCheck.py
"osVersion": winVersionText,
"x64": os.environ.get("PROCESSOR_ARCHITEW6432") == "AMD64",
"x64": os.environ.get("PROCESSOR_ARCHITEW6432") in ("AMD64", "ARM64"),
"osArchitecture": os.environ.get("PROCESSOR_ARCHITEW6432"),

@seanbudd seanbudd Aug 16, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change will require server side changes (to be recorded)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be safe to merge as-is, with changes to the server implemented later

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth raising that this is additional user tracking.

I don't think it is a big leap from tracking x64 vs x86.
The only difference is tracking which x64-bit architecture is in use, out of (AMD64, ARM64, IA64).
One potential negative - considering ARM64 and IA64 are more unusual, getting a thumbprint of a user based on their update check might become easier in theory.

@LeonarddeR LeonarddeR marked this pull request as ready for review August 16, 2022 12:19
@LeonarddeR LeonarddeR requested a review from a team as a code owner August 16, 2022 12:19
@LeonarddeR LeonarddeR requested a review from seanbudd August 16, 2022 12:19
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Aug 17, 2022
seanbudd
seanbudd previously approved these changes Aug 17, 2022
Comment thread source/updateCheck.py Outdated
@seanbudd seanbudd self-requested a review August 18, 2022 01:02
@seanbudd seanbudd merged commit af0e998 into nvaccess:master Aug 19, 2022
@nvaccessAuto nvaccessAuto added this to the 2022.4 milestone Aug 19, 2022
@LeonarddeR LeonarddeR deleted the environment branch August 23, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants