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

Announce the correct line when on the end of a link at the end of a list item in a contenteditable #11606

Merged
merged 9 commits into from Sep 16, 2020

Conversation

michaelDCurran
Copy link
Member

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.

  • I like NVDA
  • I like cats

or this html fragment:

data:text/html,<div contenteditable="true"><ul><li>I like <a href="http://www.nvaccess.org/">NVDA</a<</li><li>I like cats</li></ul></div>

This behavior is caused by code in source/NVDAObjects/IAccessible/ia2TextMozilla.py, line 97. It is commented:

# The caret is at the end of an inline object.
# This will report "blank", but we want to report the character just after the caret.

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:

  • Created a list in Google Docs that contained a link at the end of the top item. Moved the caret to the end of the link by pressing end, and confirming the current line (top list item) was announced when reading the current line with NVDA. Before this change, the next line would be announced. Pressing end now also says "blank" rather than the first character of the next line.
  • Opened the above html fragment in Firefox, switched to focus mode, and performed the same test as in Google Docs.

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:

  • NVDA now reads the correct line when the caret is placed at the end of a link on the end of a list item in Google Docs.

…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.
@AppVeyorBot
Copy link

See test results for failed build of commit b662c7fb33

feerrenrut
feerrenrut previously approved these changes Sep 15, 2020
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@michaelDCurran
Copy link
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
feerrenrut previously approved these changes Sep 16, 2020
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, a minor comment clarification that you could make, but happy for you to go ahead.

tests/system/robot/chromeTests.py Outdated Show resolved Hide resolved
Co-authored-by: Reef Turner <feerrenrut@users.noreply.github.com>
@michaelDCurran michaelDCurran merged commit 33d7b64 into master Sep 16, 2020
@michaelDCurran michaelDCurran deleted the announceCorrectLineOnEndOfLink branch September 16, 2020 20:51
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Sep 16, 2020
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.

None yet

4 participants