2024.3 Final master to beta merge #16776
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #16722. Summary of the issue: When a client disconnects from a Remote Desktop session without exiting NVDA on the server, WASAPI can throw ERROR_TOO_MANY_FILES, which is not a WASAPI error code and thus not expected by the C++ code. E_NOTFOUND is also possible, probably thrown by IMMDeviceEnumerator::GetDefaultAudioEndpoint. WasapiWavePlayer.feed raises an exception, which breaks the oneCore driver because it isn't expecting exceptions. Speech is broken henceforth. Description of user facing changes Speech is no longer silent after disconnecting from and reconnecting to a Remote Desktop session. Description of development approach I changed WasapiWavePlayer.feed to log exceptions and return without raising them. I could have tweaked the C++ code to handle ERROR_TOO_MANY_FILES and E_NOTFOUND as well as the codes it already handles (AUDCLNT_E_DEVICE_INVALIDATED and AUDCLNT_E_NOT_INITIALIZED). However, I figured it was best to make this code more resilient in general by catching exceptions, especially given that WinmmWavePlayer.feed also catches exceptions.
sslpoint.com/new-private-key-storage-requirement-for-standard-code-signing-certificates New requirements for Authenticode signing certificates which require a hardware token Description of user facing changes None - only developers (see below) Description of development approach SCons build pipeline modified to use SignPath's code signing pipeline
…longer necessary. (#16746) Reverts #10707. Summary of the issue: Firefox moved most iframes into different processes (AKA out-of-process iframes or project "Fission") in 2020. This necessitated some changes to NVDA because with Firefox's older multi-process architecture, objects in one process were unaware of objects in another. However, with the new "Cache the World" architecture released in Firefox 113 (just over a year ago in May 2023), this special handling is no longer necessary, since all objects are served from the parent process and the parent process is aware of all of them. While removing this code likely doesn't have any discernible user benefit, I think it's worthwhile to remove unnecessary complexity. There is no supported version of Firefox (even ESR) which depends on this code. Description of user facing changes None. There might be a slight performance benefit, but I doubt this is perceivable. Description of development approach Reverted #10707, adjusting for non-substantive changes that were made since #10707 was merged (code style, moved constants, etc.). Note that this moves back to using accChild instead of getNVDAObjectFromEvent. At the time, there didn't appear to be a performance benefit to using accChild and using getNVDAObjectFromEvent made it easier to implement #10707. However, I have since then discovered (through Firefox profiling) that at least in-process, AccessibleObjectFromWindow can be rather slow relative to direct COM calls. This probably isn't noticeable for NVDA's use case, especially given that we're doing this across processes, but any performance boost is probably worthwhile here. I didn't add a change log entry because there is no user visible change here
… at the end of a line. (#16745) Fixes #3156. Strictly speaking, that issue covers other problems as well, but this addresses the issue for which it was originally opened. Other issues should be (re)opened for any remaining problems. Summary of the issue: When editing text, there is an insertion point at the end of a line. This is where you are positioned when you press the end key. Firefox also allows you to reach this position with the left/right arrow keys. When text wraps across multiple lines, lines other than the last have this insertion point, but it does not have its own IAccessible2 offset, since it doesn't actually exist in the text. Instead, the caret offset reported is the same offset as the start of the next line. This is problematic for NVDA because when the user is at this position, NVDA reports the character at the start of the next line. Worse, it reports the next word and line instead of the current word and line. Description of user facing changes In Mozilla Firefox, NVDA now correctly reports the current character, word and line when the cursor is at the insertion point at the end of a line. Description of development approach IAccessible2 provides IA2_TEXT_OFFSET_CARET to deal with this. While somewhat awkward, the idea is that you pass this special offset to IAccessibleText functions so that they know you care about the caret and can handle it specially if the caret is at the end of line insertion point. For example, asking for the character at the caret might report (3, 3, None), where 3 is the caret offset. In contrast, if the caret was at the start of the next line, it might report (3, 4, 'c'). Similarly, asking for the line at the caret when at the end of the line might report (0, 3, 'ab ') instead of (3, 6, 'cd ') at the start of the next. Unfortunately, making NVDA use IA2_TEXT_OFFSET_CARET when fetching units is tricky. IA2TextTextInfo would need to keep track of the fact that it was constructed for the caret and pass IA2_TEXT_OFFSET_CARET if so. But this means that when the caret moves, the TextInfo moves too, which breaks detaching the review cursor from the caret, etc. Instead, a flag _isEndOfLineInsertionPoint has been added to MozillaCompoundTextInfo. This is set by querying the character at IA2_TEXT_OFFSET_CARET when constructing with POSITION_CARET. This allows us to implement special behaviour when this is True: we expand to no character, we expand to the current word/line instead of the next word/line, etc. If the TextInfo is copied, this flag is copied too. If the TextInfo is mutated in any way, the flag is set to False. This allows us to represent this position (even across copies) without being attached to the current position of the caret. Some of this could theoretically be done in IA2TextTextInfo, but that would have meant plumbing (and coupling) this special casing through both classes. I'm not aware of any other implementation which supports IA2_TEXT_OFFSET_CARET that doesn't use MozillaCompoundTextInfo. Note that although Chromium supports IA2_TEXT_OFFSET_CARET, it doesn't special case the insertion point at the end of a line. Thus, although this works fine in Chromium, there is no benefit there, though Chromium could choose to implement this to gain that benefit.
Part of #12387 Summary of the issue: The NVDA code base uses a mix of CRLF and LF style line endings, with the preference being CRLF. LF style endings have been introduced prior to lint checking, and can causes issues when moving code or making changes. Many IDEs will normalize the line endings when editing files, causing larger than intended diffs of files with mixed line endings. Instead, we should normalize all line endings of text files to LF and ensure that all files are committed with LF line endings. Specific files should be excluded from normalization such as: files used by translators, as we are uncertain of the side-effects here binary files git allows independent configurations between line endings used when a developer checkouts out code locally, and when code is committed to a repository. IDEs are often better suited for different line endings, with LF line endings becoming more and more standardised. However, NVDA is Windows based development, where CRLF line ending usage is commonly default or compulsory. Developers can checkout code locally, using whatever line endings they prefer or their IDE requires, using git config.core autocrlf. We can configure what line endings we commit to the repository with .gitattributes. This is set on a repository level. As what line endings we commit with is agnostic to what is checked out, the decision is based on what works best for the repository. Unix style line endings usage and support continues to increase on Windows and LF files. LF tends to be better supported by Unix style developer tools such as git. LF uses less data than CRLF.
Follow up to #12816 Closes #12387 Must be merge commit Performed the following: git add --renormalize . git commit -m "normalize line endings" Added the commit to .git-blame-ignore-revs Testing Confirmed testing by ensuring the diff is empty git diff --ignore-cr-at-eol origin/master normalize-all-line-endings
…evious character is a line feed. (#16763) Fixup of #16745. Summary of the issue: In my work on #16745, I neglected to consider an empty line after a line feed. In that case, adjusting for the line end caused NVDA to read from the previous line instead of the empty one. Description of user facing changes In Firefox, NVDA no longer speaks the previous line when the caret is on the last line and the last line is empty. However, this does not need a change log entry because this bug never shipped in release. Description of development approach Don't set the _isInsertionPointAtEndOfLine flag to True if the previous character is a line feed. This prevents the line end adjustment. Because this additional check makes the code a little more complex, I refactored this code into a helper method.
Fixes #16735 Summary of the issue: This adds Turkish grade 2 braille table to NVDA interface. Description of user facing changes Users will be able to use the table provided in NVDA Description of development approach Modified the brailleTables.py.
Fixes #14817 Summary of the issue: Our linting system has the following issues: we are stuck on an old version of flake8, as the flake8tabs module is no longer being updated we can't perform automatic lint fixes Ruff is becoming more maintained than flake8 Description of user facing changes The entire repository is linted with Ruff, rather than diffs Developers can now automatically fix certain linting issues with runlint.bat If using pre-commit hooks, linting is automatically applied: pre-commit.com/#usage CodeRabbit now scans code using our Ruff config Description of development approach integrate Ruff, migrate flake8 config Update AppVeyor scripts to use ruff add pre commit hook support for ruff update linting docs
Lint the repository with Ruff This is a follow up for #16751 Must be merge commit not squash merge Using #16751, the following commands were applied: ruff check --fix to apply automatic fixes ruff check --add-noqa to add no-qa comments to all lint failures, so that issues that can't be automatically fixed don't trigger future failures
seanbudd
requested review from
Qchristensen and
michaelDCurran
and removed request for
a team
|
Warning Review failedThe pull request is closed. WalkthroughThe overall changes aim to improve and standardize coding practices, enhance documentation, and streamline various build and testing scripts. Key tasks include enabling the Changes
Sequence Diagram(s)N/A Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
seanbudd
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Final merge for 2024.3 release, using commit
ba04b511ecc9321b180eed3ee50f3c807d6518e1from master.Changes intended for 2024.3 must now set base/target to the beta branch
*Merge not squash
Summary by CodeRabbit
New Features
rufflinter tool for improved code quality checks.ruff-pre-committool.Bug Fixes
Documentation
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdfor better clarity and readability.Chores
.gitattributesfor better file normalization and diffing.appveyor-tools/to.gitignore.Refactor
decryptFilesForSigning.ps1script by removing unnecessary certificate decryption steps.setSconsArgs.ps1to includeapiSigningTokenfor better feature signing.Style