Fix math not being read in Chromium-based browsers (#17421) #17528
Conversation
…ange that causes a call to (correctly) return E_NOTIMPL. This simple fix catches the exception and moves on. The behavior should be the same as before the Chromium change. Only E_NOTIMPL is caught, other COMErrors are re-raised. Fixes nvaccess#17421
|
The issue is quite serious. Though, it seems that it is fixed (at least temporarily) in Chrome 133. The question is When should Chrome 133 stable be released? An NVDA 2024.4.2 only makes sense if Chrome 133 stable release is not planned so soon. |
|
Maybe someone from the chrome team can comment. I just got an update and it is 131. I suspect (but again, some from Chrome should verify) that the fix needs to make it from alpha, to beta, to release and each step is about four weeks. So that puts it two months out if I am correct. |
|
Please wait and test 132.0.6834.40 |
|
@NSoiffer - please ensure NV Access can push to your fork's branch when making a contribution. I'm not sure this is possible using master as your base branch for this PR. By default, NV Access should be able to push to forks, so something is going wrong here. This PR needs to be rebased to RC, and a changelog entry added. I've pushed what this should look like here: https://github.com/nvaccess/nvda/compare/rc...fix17528?expand=1 |
SaschaCowley
added
the
conceptApproved
SaschaCowley
changed the title
|
@NSoiffer this branch should be rebased onto |
|
Are changes to the "rc" branch brought forward into master at some point or do there need to be specific commits to both rc and master? |
|
Changes in the rc branch are back merged to beta branch and beta branch is back merged in the master branch. So you to care of master branch when rebasing to rc. |
|
Hi, I advise staring over from scratch by: checkout rc branch, createa new branch form that, do the work, commit changes, push to your GitHub repo, then create a pull request with nvaccess/rc as base. This is because master and rc branches have diverged and master branch content contains code marked as API breaking (and we don’t want to see that happening in 2024.4.2). Thanks.
|
|
Closing in favour of #17548 |
Fix math not being read in Chromium-based browsers. Chromium made a change that causes a call to (correctly) return E_NOTIMPL.
This simple fix catches the exception and moves on. The NVDA behavior should be the same as before the Chromium change. Only E_NOTIMPL is caught, other COMErrors are re-raised.
Fixes #17421.
This fixes a very serious user-issue. I recommend that a 2024.4.2 build happen with this fix if possible.
Link to issue number:
Fixes #17421
Summary of the issue:
Chromium-browsers were changed to return E_NOTIMPL and broke speech and braille of all math equations (i.e., no speech or braille for math) in those browsers because NVDA passes along a 'not implemented' COM exception.
Description of user facing changes
This restores the behavior of NVDA (i.e., math reads) as before the change.
Description of development approach
A try/except block is added around the call. If the error is E_NOTIMPL, the code moves on as before. Otherwise, the error is re-raised as before.
Testing strategy:
Make sure that some math addon is present. I tested with MathCAT, but any math addon will do. Go to any page with math (e.g., https://en.wikipedia.org/wiki/Quadratic_equation). Navigate to a math expression. Without the change, you will not hear any math. With the change, you will hear the math expression being read.
Known issues with pull request:
None.
Code Review Checklist:
I did not make a change log entry because I don't know what version this change will go into. I think a simple entry like
@coderabbitai summary