Announce the correct line when on the end of a link at the end of a list item in a contenteditable#11606
Merged
Merged
Conversation
…at the end of an inline element, make sure not to search above the deepest non-inline element. This stops accidentally falling into the next paragraph or list item etc.
See test results for failed build of commit b662c7fb33 |
feerrenrut
previously approved these changes
Sep 15, 2020
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
As suggested in the description for this PR, I think it would be good to hold off on merging and also include a system test.
Member
Author
|
@feerrenrut I've added a system test to this now for you to review please. I confirmed that the system test breaks if I revert the actual change. |
feerrenrut
previously approved these changes
Sep 16, 2020
feerrenrut
left a comment
Contributor
There was a problem hiding this comment.
Looks good, a minor comment clarification that you could make, but happy for you to go ahead.
Co-authored-by: Reef Turner <feerrenrut@users.noreply.github.com>
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:
None.
Summary of the issue:
If a web page contains a contenteditable, which contains an inline element at the end of a block-level element E.g. a link at the end of a paragraph, or a span at the end of an li (list item) element,
And the caret is placed at the very end of the line, reading the line with NVDA will announce the next line, not the current. Similarly, if downArrowing to the next line and then back up again, the next line is announced, not the current.
This can be reproduced in Google Docs documents, by simply creating a list with one or more items, and placing a link as the end of the top list item.
E.g.
or this html fragment:
This behavior is caused by code in source/NVDAObjects/IAccessible/ia2TextMozilla.py, line 97. It is commented:
And then we re-fetch the caret textInfo using _findNextContent so that it is truly the character after the caret.
However, this means in the case of the caret being at the end of an inline link at the end of a line, the character is reported as the first character of the next line, and expanding to line expands to the next line, not the current.
Description of how this pull request fixes the issue:
A new limitToInline boolean keyword argument has been added to MozillaCompoundTextInfo._findNextContent, which when true, causes the method not to go above the deepest non-inline element when searching for the next object.
So in the case of a link at the end of a list item, it will not search outside of the list item.
This allows the work-around to still work at the end of links in the middle of a list item or paragraph, but no longer at the end, where it was inappropriate to do so.
Testing performed:
I can produce a system test once prs #11605 and #11569 are merged as these change the output of lists in contenteditables.
Known issues with pull request:
None.
Change log entry:
Bug fixes: