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

fixed #15897: Entering Roman numeral analysis crashes when using screen reader #16020

Merged
merged 1 commit into from Jan 26, 2023

Conversation

Eism
Copy link
Contributor

@Eism Eism commented Jan 23, 2023

Resolves: #15897

The C++ standard library's regex implementation does not support lookarounds.

@MarcSabatella
Copy link
Contributor

Thanks! This definitely prevents the crash, and reads correctly for "\b". But the same change would need to be made for "#" and "h", so "#" reads "number" rather than "backslash sharp", and "\h" reads "h" instead of "backslash natural". Actually for me using Orca natural doesn't read at all currently, but I think that's just that Orca doesn't know this Unicode symbol.

The way your code works, I don't think this technique is needed for "bb" or "##", because if you literally want "bb", you need to type \b\b. The code you have in place already handles that.

@Eism
Copy link
Contributor Author

Eism commented Jan 24, 2023

@MarcSabatella fixed

@MarcSabatella
Copy link
Contributor

Looks great, thanks for dealing with this!

@NoahTCarver
Copy link

Excellent. Thank you so much, @Eism and @MarcSabatella, for your work on this. It's greatly appreciated.

@Eism Eism merged commit fa46fff into musescore:master Jan 26, 2023
@Eism Eism deleted the score_harmony_crash_fix branch January 26, 2023 06:55
@NoahTCarver
Copy link

NoahTCarver commented Jan 27, 2023

Hello. Now using MuseScoreNightly-230270506-4.0.2-3e9f131-x86_64 and MuseScoreNightly-230270305-master-a5382f4-x86_64. The Roman numeral analysis crash has been fixed, but text entry seems not to be possible. Pressing enter on the RNA menu item seems to send focus to the text field but entry is not possible. Further, the right and left arrows continue to move focus through the menu bar. Pressing escape seems to return focus to the score as arrow keys move focus through text fields for NRA entry, however again, text entry seems not to work as once I press escape again after entering RNA, no RNA can be found in the score. Tested with latest stable JAWS and NVDA.

@MarcSabatella
Copy link
Contributor

Yes, there is still a bug where focus remains in the menu - see #15979. Pressing Esc does work, but it is possible you may have hit it too many times, or at the wrong point. Try pressing Esc exactly once, immediately after activating the RNA menu item. This works for me. But it will definitely be best to get the focus bug fixed!

@MarcSabatella
Copy link
Contributor

Also, it should work now to assign a keyboard shortcut to RNA - except unfortunately the shortcut dialog is currently not accessible, it's impossible to save the shortcut and close the dialog using the keyboard alone.

@NoahTCarver
Copy link

Here's a video demo of what I'm experiencing. I am only pressing escape once as shown in the video, but this bug still occurs. Hope this helps.

https://youtu.be/GDqIj7M1HlA

@MarcSabatella
Copy link
Contributor

Thanks for the video! It definitely seems that you are doing the correct things, and I think the screen reader feedback sounds correct. However, unfortunately, your application is not large enough to show the score - the mixer is covering it all. So I am unable to see much more about what is actually happening. However, one thing I do see in the mixer is that when you select the Roman numeral analysis item in the menu, the mixer adds a new channel for the chord symbol (which I guess is a bug since these do not actually play), so it is at least trying to add this. When you first hit Esc, the channel remains, and it does when you press "I" as well, but then when you hit Esc again, the channel is removed. This is consistent with what I'd expect if the "I" in fact did not get sent to the edit field. In that case, since no text was entered, the new analysis text would simply deleted upon hitting Esc since it is empty.

And yet when I reproduce exactly those same steps, all is well for me.

So can you perhaps do the same thing again but first make sure the mixer is not open? F10 toggles it, or you can use the View menu. You can F6 or tab around the interface to be sure it isn't open. Just that one little section starting from navigating the menu up through when you navigate through the score to show the text isn't there.

@NoahTCarver
Copy link

I tried it again tonight... and it works now both with the mixer turned on or off. There were no changes to screen reader configuration, I'm using the same build, MuseScoreNightly-230270506-4.0.2-3e9f131-x86_64, there were no system changes that I'm aware of, and I followed the same steps as outlined in the video I posted a few days ago. Put simply, it works now, and I have no idea why. Bizarre.

@Eism
Copy link
Contributor Author

Eism commented Jan 30, 2023

@NoahCarverMusic
I fixed your issue and merged to master
This is a direct link to Windows build https://github.com/musescore/MuseScore/suites/10655099799/artifacts/532549830

@NoahTCarver
Copy link

After more testing, it seems that @Eism's fix works as intended (barring the focus issues mentioned by @MarcSabatella) when MuseScore 4 launches as a maximized window or is manually maximized by the user. However, the Roman numeral input is not recognized by MS4 if the application is launched but its window is not maximized either automatically or manually. I'll try to get this on video. I'm seeing this with both JAWS and NVDA on Windows 11; unfortunately, I can't test with other operating systems at the moment.

Also, @Eism, thanks for the info. I'm aware your issue was merged to master; I really appreciate your work. Both @MarcSabatella and I are now in agreement that RNA input with a screen reader works as intended now barring what I seem to have gradually discovered over the past few days regarding MS4 not recognizing RNA input when its window is not maximized at least on Windows. I've been testing a build that (as far as I know) contains the fix given it no longer crashes when entering RNA, however I can also test using the build you just posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MU4 Issue] Entering Roman numeral analysis crashes when using screen reader
4 participants