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

[MU4 Issue] Entering Roman numeral analysis crashes when using screen reader #15897

Closed
MarcSabatella opened this issue Jan 15, 2023 · 8 comments · Fixed by #16020
Closed

[MU4 Issue] Entering Roman numeral analysis crashes when using screen reader #15897

MarcSabatella opened this issue Jan 15, 2023 · 8 comments · Fixed by #16020
Assignees
Labels
P0 Priority: Critical

Comments

@MarcSabatella
Copy link
Contributor

Describe the bug
Enter RNA text crashes MuseScore if a sxcreen reader isrunning

To Reproduce
Steps to reproduce the behavior:

  1. New score (be sure to have a screen reader running)
  2. Select first rest
  3. Add / Text / Roman numeral analysis
  4. Type I
  5. Press Esc
  6. See error: crash

Expected behavior
The text is entered normally

Screenshots
If applicable, add screenshots to help explain your problem.

Platform information
Confirmed using NVDA, JAWS, and Narrator on Windows, also Orca on Linux. In some cases apparently the crash might happen sooner, like right after step 3, depending on how quickly you type I guess.

See https://musescore.org/en/node/342415 for more info

@NoahTCarver
Copy link

NoahTCarver commented Jan 15, 2023

In some cases apparently the crash might happen sooner, like right after step 3, depending on how quickly you type I guess.

See https://musescore.org/en/node/342415 for more info

Hi. Some clarification to your comment above. For me, the crash occurs immediately after invoking the "Roman numeral analysis" menu item. No Roman numeral entry is necessary to cause the crash in my case.

@MarcSabatella
Copy link
Contributor Author

Using orca, the following gets printed to the console in the window I started orca from (not the window I started MuseScore from):

(orca:7559): dbind-CRITICAL **: 13:17:59.426: atspi_accessible_set_cache_mask: assertion 'accessible->parent.app != NULL' failed

Which is probably a good clue.

@shoogle shoogle added this to Needs Triage in [MU4.0.2 PATCH-RELEASE] via automation Jan 20, 2023
@shoogle shoogle moved this from Needs Triage to Important in [MU4.0.2 PATCH-RELEASE] Jan 20, 2023
@shoogle shoogle added the P0 Priority: Critical label Jan 20, 2023
@Eism Eism assigned Eism and unassigned vpereverzev Jan 20, 2023
@Eism
Copy link
Contributor

Eism commented Jan 20, 2023

@MarcSabatella
I tested it on Linux with Orca.
The crash occurs on this line harmony.cpp#L2124 with

terminate called after throwing an instance of 'std::regex_error'
  what():  Invalid special open parenthesis.

You implemented it in this PR #5459

The regex looks invalid and the crash started to occur after replacing QRegularExpression with std::regex.

Can you please explain how regex was supposed to work?
And
Do we really need regex here, or can we just use a replacement like for rnaReplacements?

@MarcSabatella
Copy link
Contributor Author

MarcSabatella commented Jan 20, 2023

Thanks for the analysis! As I recall, the overall point of the change was to make the screen understand to read "five" (translated as appropriate) instead of "vee" when reading the Roman number "V", and similarly for "flat" instead of "bee" for "b", etc. However, in RNA, the letter "b" can sometimes mean the letter "b" and not flat. This is accomplished while typing the RNA by preceding the "b" with a backslash. Backslash can also be used to escape other symbols that are otherwise interpreted specially.

So the point of this particular section of code was supposed to be, to only match and perform the substitution (e.g., replacing "b" with the actual Unicode flat sign) if the first symbol is not preceded by a backslash. I'm not an expert in regular expressions and probably arrived at what I did through studying the Qt documentation specifically and also some trial and error, so no great surprise if it only worked accidentally in Qt and causes problems in a different implementation.

Doing a little digging, I think this was meant to be an example of a "negative lookbehind assertion" as per https://www.regular-expressions.info/lookaround.html. I doubt I had ever heard of that before writing this code, and I must have instantly forgot anything I ever learned about it once I got it working, because I have no recollection of this anymore! But clearly, that was the intent - I wanted to match "b not preceded by backslash". So, hopefully there is a way to do that still.

BTW, If I'm reading my code correctly, it seems like it will actually read the backslash if present. That's not good. I'd want "b" to read "flat" and "\b" to read "bee" (not "backslash bee").

If you're able to fix this, that would be fantastic!

@MarcSabatella
Copy link
Contributor Author

BTW, a brute force non-regex solution might be, first just replace all the b's with flat (♭) normally, but then do a second pass and replace occurrences of "\♭" with the letter b again.

@NoahTCarver
Copy link

NoahTCarver commented Jan 21, 2023

Here's a debug log from NVDA detailing what occurs from NVDA's perspective. This snippet begins immediately after I press enter on the "Roman numeral analysis" menu item. The full log is also attached. Finally, simply restarting NVDA after launching MuseScore is enough to prevent the crash when using Nightly-230190506-4.0.1-9b70a8c-x86_64, however this does not work in version 4.0.1.230121751 revision 9b70a8c. This log was taken while using MuseScore version 4.0.1.230121751 revision 9b70a8c.

DEBUG - editableText.EditableText._hasCaretMoved (01:59:30.928) - MainThread (4040):
Focus event. Elapsed: 20 ms
DEBUGWARNING - NVDAObjects.behaviors.EditableText._reportErrorInPreviousWord (01:59:30.938) - MainThread (4040):
Error fetching last character of previous word
Traceback (most recent call last):
  File "NVDAObjects\UIA\__init__.pyc", line 427, in __init__
  File "monkeyPatches\comtypesMonkeyPatches.pyc", line 32, in __call__
_ctypes.COMError: (-2147220991, 'An event was unable to invoke any of the subscribers', (None, None, None, 0, None))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "NVDAObjects\behaviors.pyc", line 191, in _reportErrorInPreviousWord
  File "documentBase.pyc", line 74, in makeTextInfo
  File "NVDAObjects\UIA\__init__.pyc", line 429, in __init__
RuntimeError: No selection available
DEBUGWARNING - NVDAObjects.UIA.UIA._prefetchUIACacheForPropertyIDs (01:59:30.940) - MainThread (4040):
IUIAutomationElement.buildUpdatedCache failed given IDs of {30019, 30086, 30022, 30025, 30155, 30138, 30036, 30070, 30103, 30008, 30009, 30010, 30046, 30079}
DEBUGWARNING - eventHandler.executeEvent (01:59:30.941) - MainThread (4040):
error executing event: typedCharacter on <NVDAObjects.Dynamic_MenuItemEditableTextWithAutoSelectDetectionUIA object at 0x0819B830> with extra args of {'ch': '\r'}
Traceback (most recent call last):
  File "eventHandler.pyc", line 300, in executeEvent
  File "eventHandler.pyc", line 101, in __init__
  File "eventHandler.pyc", line 110, in next
  File "NVDAObjects\behaviors.pyc", line 236, in event_typedCharacter
  File "NVDAObjects\__init__.pyc", line 1143, in event_typedCharacter
  File "speech\speech.pyc", line 1106, in speakTypedCharacters
  File "api.pyc", line 337, in isTypingProtected
  File "baseObject.pyc", line 62, in __get__
  File "baseObject.pyc", line 168, in _getPropertyViaCache
  File "NVDAObjects\__init__.pyc", line 1027, in _get_isProtected
  File "baseObject.pyc", line 62, in __get__
  File "baseObject.pyc", line 168, in _getPropertyViaCache
  File "NVDAObjects\UIA\__init__.pyc", line 1619, in _get_states
  File "NVDAObjects\UIA\__init__.pyc", line 985, in _getUIACacheablePropertyValue
  File "monkeyPatches\comtypesMonkeyPatches.pyc", line 32, in __call__
_ctypes.COMError: (-2147220991, 'An event was unable to invoke any of the subscribers', (None, None, None, 0, None))
DEBUGWARNING - eventHandler.executeEvent (01:59:31.130) - MainThread (4040):
error executing event: gainFocus on <NVDAObjects.Dynamic_EditableTextWithAutoSelectDetectionUIA object at 0x081AB6D0> with extra args of {}
Traceback (most recent call last):
  File "eventHandler.pyc", line 300, in executeEvent
  File "eventHandler.pyc", line 101, in __init__
  File "eventHandler.pyc", line 110, in next
  File "C:\Users\noahc\AppData\Roaming\nvda\addons\remote\globalPlugins\remoteClient\__init__.py", line 465, in event_gainFocus
    nextHandler()
  File "eventHandler.pyc", line 110, in next
  File "C:\Users\noahc\AppData\Roaming\nvda\addons\BrailleExtender\globalPlugins\brailleExtender\__init__.py", line 243, in event_gainFocus
    nextHandler()
  File "eventHandler.pyc", line 110, in next
  File "NVDAObjects\behaviors.pyc", line 247, in event_gainFocus
  File "NVDAObjects\UIA\__init__.pyc", line 1316, in event_gainFocus
  File "NVDAObjects\__init__.pyc", line 1198, in event_gainFocus
  File "NVDAObjects\__init__.pyc", line 1095, in reportFocus
  File "speech\speech.pyc", line 609, in speakObject
  File "speech\speech.pyc", line 649, in getObjectSpeech
  File "speech\speech.pyc", line 501, in getObjectPropertiesSpeech
  File "baseObject.pyc", line 62, in __get__
  File "baseObject.pyc", line 168, in _getPropertyViaCache
  File "NVDAObjects\UIA\__init__.pyc", line 1581, in _get_keyboardShortcut
  File "NVDAObjects\UIA\__init__.pyc", line 985, in _getUIACacheablePropertyValue
  File "monkeyPatches\comtypesMonkeyPatches.pyc", line 32, in __call__
_ctypes.COMError: (-2147220991, 'An event was unable to invoke any of the subscribers', (None, None, None, 0, None))

RNA Crash NVDA Debug.log

Eism added a commit to Eism/MuseScore that referenced this issue Jan 23, 2023
Eism added a commit to Eism/MuseScore that referenced this issue Jan 23, 2023
@Eism
Copy link
Contributor

Eism commented Jan 23, 2023

BTW, a brute force non-regex solution might be, first just replace all the b's with flat (♭) normally, but then do a second pass and replace occurrences of "\♭" with the letter b again.

Done.
Please test the build from #16020

@RomanPudashkin RomanPudashkin moved this from Important to In progress in [MU4.0.2 PATCH-RELEASE] Jan 24, 2023
Eism added a commit to Eism/MuseScore that referenced this issue Jan 24, 2023
[MU4.0.2 PATCH-RELEASE] automation moved this from In progress to Done Jan 26, 2023
Eism added a commit that referenced this issue Jan 26, 2023
fixed #15897: Entering Roman numeral analysis crashes when using screen reader
Eism added a commit to Eism/MuseScore that referenced this issue Jan 26, 2023
RomanPudashkin added a commit that referenced this issue Jan 26, 2023
fixed #15897: Entering Roman numeral analysis crashes when using screen reader
octopols pushed a commit to octopols/MuseScore that referenced this issue Jan 31, 2023
@abariska
Copy link

abariska commented Mar 5, 2023

🟢 FIXED
Tested on macOS 12.6

@abariska abariska moved this from Done to Done - Tested Again in [MU4.0.2 PATCH-RELEASE] Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 Priority: Critical
Projects
No open projects
[MU4.0.2 PATCH-RELEASE]
Done - Tested Again
Development

Successfully merging a pull request may close this issue.

6 participants