Don't use IA2TextTextInfo on the web#20132
Closed
jcsteh wants to merge 4 commits into
Closed
Conversation
…w and read line. (nvaccess#20096)" This reverts most of commit 3ad497a. It does not revert the type annotation fixes in review.py or the fix entry in changes.md.
Contributor
Author
|
This currently completely breaks text editing in web browsers. Drafting until I figure that out. |
Contributor
Author
|
Ah Zarq, I just realised we're now in 2026.3, but #20096 was in 2026.2. So I guess this needs to be rebased against beta. |
Contributor
Author
|
Actually, I think it might be better to revert #20096 from beta and let this whole thing wait until 2026.3. |
This was referenced May 14, 2026
Contributor
Author
|
Arrrrgh! Changing the TextInfo like this breaks mouse tracking. Mouse tracking's handling of embedded objects is all kinds of broken and probably shouldn't use IA2TextTextInfo the way it does, but that's a much bigger lift to fix, so we're going to need yet another alternative here. |
Contributor
Author
seanbudd
pushed a commit
that referenced
this pull request
May 15, 2026
#20134) ### Reverts PR Reverts #20096. ### Issues fixed Fixes #20096 (comment) and #20096 (comment). ### Issues reopened Reopens #15159. ### Reason for revert The implementation in #20096 still reviewed the wrong content if you focused a control by pressing tab in focus mode or if you used the review top/bottom line commands. I submitted #20132, but given the proximity to beta, it seems safer to wait for 2026.3 instead. ### Can this PR be reimplemented? If so, what is required for the next attempt See #20132.
This file contains hidden or 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
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.
Link to issue number:
Follow-up to #20096, which was a fix for #15159.
Summary of the issue:
As noted in #20096 (comment), the implementation in #20096 still reviewed the wrong content if you focused a control by pressing tab in focus mode or if you used the review top/bottom line commands.
Description of user facing changes:
Description of development approach:
I reverted the functional changes in #20096, opting for a different approach. Rather than altering how read line and review fetch the TextInfo using an attribute, Ia2Web.TextInfo is now directly set to NVDAObjectTextInfo, ensuring that makeTextInfo will always use that for Ia2Web objects.
I didn't do this initially because by itself, it breaks script_reportDetailsSummary. However, it turns out that script_reportDetailsSummary had some incorrect assumptions and contained code that was intended to execute but never did. It assumed that browsers always set the caret even for non-editable controls, but that isn't always true, especially in Firefox. Even though it had code to fall back to using the focus, it returned early before that was ever executed. I fixed those problems, which actually fixes script_reportDetailsSummary in Firefox in focus mode on non-editable controls.
I also discovered that MozillaCompoundTextInfo explicitly checked for cases where an embedded object's TextInfo was NVDAObjectTextInfo. However, this only applied to images, etc. which don't support IAccessibleText, as per the IAccessible.TextInfo implementation. So, I simply made this check more explicit: it now checks for IAccessibleText and uses IA2TextTextInfo in that case, NVDAObjectTextInfo otherwise.
Testing strategy:
All the tests described for #20096, plus, in both Firefox and Edge, with this test case:
data:text/html,<button aria-label="1"></button><button aria-label="2"></button><button aria-details="d">3</button><p id="d">d</p><div contenteditable role="textbox">a<span aria-details="d">b</span></div>Known issues with pull request:
I looked for other non-obvious instances where we rely on the previous behaviour internally like script_reportDetailsSummary or MozillaCompoundTextInfo, but didn't find any. However, I can't be certain that I didn't miss anything else.
Code Review Checklist: