Several fixes to native selection mode by michaelDCurran · Pull Request #16129 · nvaccess/nvda

michaelDCurran · 2024-02-05T03:31:33Z

Link to issue number:

Fixes #16064
Fixes #16097

Summary of the issue:

Native selection mode can be enabled in unsupported Gecko, such as Thunderbird 115.x #16064: turning on native selection mode in thunderbird 115 would cause errors when moving with the arrow keys as IAccessibleTextSelectionContainer is unavailable.
Wrong message reported when toggling native selection in Word #16097: NVDA is misleading in MS Word browse mode when it says that native selection mode is not supported when trying to toggle it on, as MS word Browse mode does move the caret / selection, thus it has always used native selection mode.. It is just you can't toggle it off.

Description of user facing changes

In firefox, If NVDA fails to update the native selection when turning on native selection mode, it is now left off, and the user is
notified that native selection mode is not supported. this stops errors when moving with the arrow keys in Thunderbird after turning on native selection mode.
NVDA no longer incorrectly alerts the user that native selection mode is not supported in Microsoft Word. Rather, the message state that it canot be turned off.
when copying text with control+c in Microsoft Word with Browse mode on, formatting is now also copied, fulfilling the expectation of a native selection mode.

Description of development approach

Gecko virtualBuffer's updateAppSelection method: if the selection is collapsed, don't try fetching information for the selection, instead just clear the app selection. This error was previously silently ignored.
In UIA Browse mode documents, _nativeAppSelectionMode is now set to True, as this correctly reflects that UIA browse mode documents move the native selection.
BrowseMode document's toggleNativeSelectionMode script:
- Tailor the message reported if native selection mode is not supported (by looking at _nativeSelectionMode). If _nativeSelectionMode is True, then the message is changed to state it can't be turned off.
- If updateappSelection fails when turning on native selection mode, log the error, turn it off, and alert the user that it is not supported.
In MS word, suppress the "copy" UIA notification, if in MS word Browse mode. Otherwise there would be double speaking along with the message in cursorManager's copyToclipboard script when doing a native copy.

Testing strategy:

Opened a page in Firefox. Turned on native selection mode. Selected some text, copied, and pasted in to MS Word to confirm that the text was formatted.
Tried togling native sleection mode in MS word browse mode. Confirmed that NvDA reported that it could not be toggled off.
Temporarily caused Gecko virtualbuffer's updateAppSelection method to raise NotImplementedError, Opened a page in Firefox, and tried turning on native selection mode. Confirmed that NVDA reported it was not supported.

Known issues with pull request:

None known.

Code Review Checklist:

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

…ing: * In firefox, If NVDA fails to update the native selection when turning on native selection mode, it is now left off, and the user is notified that native selection mode is not supported. this stops errors when moving with the arrow keys in Thunderbird after turning on native selection mode. * Exceptions are logged when turning on and off native selection mode. * If native selection mode is not supported when trying to turn it on, NVDA honors the _nativeAppSelectionMode boolean to tailor the message to state that it cannot be turned off, if it is on (though not supported). * UIA browse mode documents (E.g. MS word) set _nativeSelectionMode to True, though it cannot be turned off. This not only provides a better message to the user if they try and toggle it, but it also has the advantage that copying with control+c from MS word browse mode, will now also copy with formatting.

AppVeyorBot · 2024-02-05T04:49:22Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/q6dh8ex9bqfq12g2/artifacts/output/nvda_snapshot_pr16129-30947,e8102767.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.0,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 28.7,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 2.3,
FINISH_END 0.6

See test results for failed build of commit e81027670d

…, as the methods already convert ot to NotImplementedError.

Co-authored-by: Sean Budd <sean@nvaccess.org>

CyrilleB79

Please, also update the change log to indicate that native copy (i.e. with formatting) is now done in Word Browse Mode. Before, in Word Browse Mode, formatting was copied when using Object Model, but not when using UIA.
Thanks.

CyrilleB79 · 2024-02-05T09:43:45Z

+				# Translators: the message when native selection mode is not available in this browse mode document.
+				ui.message(_("Native selection mode unsupported in this browse mode document"))
+			else:
+				# Translators: the message when native selection mode cannot be turned off in this browse mode document.
+				ui.message(_("Native selection mode cannot be turned off in this browse mode document"))


"browse mode document" -> "document"

My feeling is that, for users and translators, there is no need to specify here that the document is a browse mode document (no possible ambiguity). This way, the messages are lighter. Feel free to ignore this suggestion if you disagree with me though.

Suggested change

# Translators: the message when native selection mode is not available in this browse mode document.

ui.message(_("Native selection mode unsupported in this browse mode document"))

else:

# Translators: the message when native selection mode cannot be turned off in this browse mode document.

ui.message(_("Native selection mode cannot be turned off in this browse mode document"))

# Translators: the message when native selection mode is not available in this document.

ui.message(_("Native selection mode unsupported in this document"))

else:

# Translators: the message when native selection mode cannot be turned off in this document.

ui.message(_("Native selection mode cannot be turned off in this document"))

…ult a and cannot be turned off, so ensure the toggle message reflects this. (#16151) Follow up from #16129 Fixes #16097 Summary of the issue: Selecting text in Browse mode in Microsoft Word moves the physical selection. Thus it is the equivalent to native selection mode. Pr #16129 ensured that Browse mode in MS word with UI Automation took this into account and made the toggle message report that native selection mode could not be turned off. However this has not been done for MS Word object model, and NVDA states that native selection mode is unsupported. Description of user facing changes When trying to toggle native selection mode in Browse mode in MS Word when not using UI Automation, NvDA will correctly state that native selection mode cannot be turned off. When copying text with control+c in browse mode in MS Word when not using UI Automation, formatting is now also copied. Description of development approach Set browse mode for MS Word object model's _nativeAppSelectionMode to True.

Fixes nvaccess#16064 Fixes nvaccess#16097 Summary of the issue: Native selection mode can be enabled in unsupported Gecko, such as Thunderbird 115.x nvaccess#16064: turning on native selection mode in thunderbird 115 would cause errors when moving with the arrow keys as IAccessibleTextSelectionContainer is unavailable. Wrong message reported when toggling native selection in Word nvaccess#16097: NVDA is misleading in MS Word browse mode when it says that native selection mode is not supported when trying to toggle it on, as MS word Browse mode does move the caret / selection, thus it has always used native selection mode.. It is just you can't toggle it off. Description of user facing changes In firefox, If NVDA fails to update the native selection when turning on native selection mode, it is now left off, and the user is notified that native selection mode is not supported. this stops errors when moving with the arrow keys in Thunderbird after turning on native selection mode. NVDA no longer incorrectly alerts the user that native selection mode is not supported in Microsoft Word. Rather, the message state that it canot be turned off. when copying text with control+c in Microsoft Word with Browse mode on, formatting is now also copied, fulfilling the expectation of a native selection mode. Description of development approach Gecko virtualBuffer's updateAppSelection method: if the selection is collapsed, don't try fetching information for the selection, instead just clear the app selection. This error was previously silently ignored. In UIA Browse mode documents, _nativeAppSelectionMode is now set to True, as this correctly reflects that UIA browse mode documents move the native selection. BrowseMode document's toggleNativeSelectionMode script: Tailor the message reported if native selection mode is not supported (by looking at _nativeSelectionMode). If _nativeSelectionMode is True, then the message is changed to state it can't be turned off. If updateappSelection fails when turning on native selection mode, log the error, turn it off, and alert the user that it is not supported. In MS word, suppress the "copy" UIA notification, if in MS word Browse mode. Otherwise there would be double speaking along with the message in cursorManager's copyToclipboard script when doing a native copy.

…ult a and cannot be turned off, so ensure the toggle message reflects this. (nvaccess#16151) Follow up from nvaccess#16129 Fixes nvaccess#16097 Summary of the issue: Selecting text in Browse mode in Microsoft Word moves the physical selection. Thus it is the equivalent to native selection mode. Pr nvaccess#16129 ensured that Browse mode in MS word with UI Automation took this into account and made the toggle message report that native selection mode could not be turned off. However this has not been done for MS Word object model, and NVDA states that native selection mode is unsupported. Description of user facing changes When trying to toggle native selection mode in Browse mode in MS Word when not using UI Automation, NvDA will correctly state that native selection mode cannot be turned off. When copying text with control+c in browse mode in MS Word when not using UI Automation, formatting is now also copied. Description of development approach Set browse mode for MS Word object model's _nativeAppSelectionMode to True.

michaelDCurran requested a review from a team as a code owner February 5, 2024 03:31

michaelDCurran requested review from SaschaCowley and removed request for a team February 5, 2024 03:31

seanbudd added this to the 2024.1 milestone Feb 5, 2024

seanbudd previously approved these changes Feb 5, 2024

View reviewed changes

Comment thread source/browseMode.py Outdated

Browse mode Toggle native selection script: no need to catch COMError…

442ee95

…, as the methods already convert ot to NotImplementedError.

michaelDCurran dismissed seanbudd’s stale review via 442ee95 February 5, 2024 05:37

Update source/browseMode.py

6a89e65

Co-authored-by: Sean Budd <sean@nvaccess.org>

CyrilleB79 reviewed Feb 5, 2024

View reviewed changes

Update what's new

9d88619

seanbudd approved these changes Feb 6, 2024

View reviewed changes

seanbudd merged commit 29c0d72 into beta Feb 6, 2024

seanbudd deleted the i16064 branch February 6, 2024 00:24

CyrilleB79 mentioned this pull request Feb 8, 2024

Wrong message reported when toggling native selection in Word #16097

Closed

michaelDCurran mentioned this pull request Feb 8, 2024

MS Word object model browse mode: ensure native selection mode toggle message reflects that it cannot be turned off #16151

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Several fixes to native selection mode#16129

Several fixes to native selection mode#16129
seanbudd merged 4 commits into
betafrom
i16064

michaelDCurran commented Feb 5, 2024

Uh oh!

AppVeyorBot commented Feb 5, 2024

Uh oh!

Uh oh!

CyrilleB79 left a comment

Uh oh!

CyrilleB79 Feb 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

michaelDCurran commented Feb 5, 2024

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

AppVeyorBot commented Feb 5, 2024

Uh oh!

Uh oh!

CyrilleB79 left a comment

Choose a reason for hiding this comment

Uh oh!

CyrilleB79 Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants