Skip to content

Announce entering a listItem when moving by character or word in contentEditables and browse mode #11569

Merged
michaelDCurran merged 9 commits into
masterfrom
listItemAsMarker
Sep 16, 2020
Merged

Announce entering a listItem when moving by character or word in contentEditables and browse mode #11569
michaelDCurran merged 9 commits into
masterfrom
listItemAsMarker

Conversation

@michaelDCurran

Copy link
Copy Markdown
Member

Link to issue number:

None.

Summary of the issue:

When inside a list in a contentEditable on the web, cursoring from one listItem to the next does not provide any speech feedback telling you that you are in a new listItem. In browse mode, at least you'd probably have a bullet or number which would alert you to the fact, but in contentEditables, usually the cursor skips over the bullet.
Take this example in Chrome / Firefox.

data:text/html,<div contenteditable="true"><ul><li>This is bullet one</li><li>This is bullet two</li></ul></div> 

NVDa should speak "list item" when cursoring with left and right arrow from the first to the second, or the second to the first. Note that we should not do this when moving by line, nor should we say "out of list item" as that is somewhat redundant.

Description of how this pull request fixes the issue:

  • TextInfo.getPresentationCategory now takes an 'extraDetail' keyword argument.
  • TextInfo.getPresentationCategory treats ROLE_LISTITEM as a marker (rather than layout) if extraDetail is true.
  • speech.getControlFieldSpeech now passes its extraDetail argument to TextInfo.getPresentationCategory.
  • Provided an implementation of _getcontrolFieldForObject on MozillaCompoundTextInfo, which sets the 'uniqueID' key on the controlField using IAccessible2 uniqueID if available. This ensures that controlFields from different objects (no matter if they have the same role and other attributes) are treated as different controlFields when calculating entering / exiting controlFields in speech. Without this change, entering a new listItem would still say nothing as both controlFields look identical.

these changes together mean that NvDA will now announce 'listItem' when moving into a listItem when navigating by character or word.

Testing performed:

With the above testcase in Firefox / chrome: in focus mode ensured that pressing left and right arrow crossing the list item boundary speaks "list item" but not "out of list item" and ensure that moving by line does not announce "list item".

Known issues with pull request:

When arroing into a list from the outside with the left or right arrows, NVDA will say "list list item". Technically this is correct, but perhaps a little redundant. However, as this only happens when moving by word or character, I don't believe this is too annoying, weighing it up against what this does fix.

Change log entry:

Bug fixes:

  • When arrowing by character or word from one list item to another in editable content on the web, entering the new list item is now announced.

@michaelDCurran michaelDCurran added this to the 2020.4 milestone Sep 6, 2020
@AppVeyorBot

This comment has been minimized.

LeonarddeR
LeonarddeR previously approved these changes Sep 7, 2020
@AppVeyorBot

This comment has been minimized.

@michaelDCurran

Copy link
Copy Markdown
Member Author

This pr now depends on pr #11581.

Comment thread tests/system/robot/chromeTests.py
Comment thread tests/system/robot/chromeTests.py
@AppVeyorBot

This comment has been minimized.

@michaelDCurran

Copy link
Copy Markdown
Member Author

@feerrenrut I have improved the test so that it actually uses a contentEditable in focus mode, and also tests moving by word (as its name suggests it should).
And now that pr #11581 has been merged, I have rebased this again.

feerrenrut
feerrenrut previously approved these changes Sep 15, 2020
Comment thread tests/system/robot/chromeTests.py Outdated
Comment on lines +58 to +61
# Move focus into the content editable.
# This will turn on focus mode.
# And the caret will be on the paragraph before the list.
spy.emulateKeyPress("tab")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should use the option to speak focus/browse mode and check for it here?

Comment thread tests/system/robot/chromeTests.py Outdated
_asserts.strings_match(
actualSpeech,
"list item level 1 \nz"
"list item level 1 \nb"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It would be good if we could make this easier to read. I assume that the new line was added to separate utterances.

Would it be more clear if written like this?

Suggested change
"list item level 1 \nb"
"\n".join([
"list item level 1 ",
"b"
])

@michaelDCurran

Copy link
Copy Markdown
Member Author

@feerrenrut hopefully one last review please. Focus mode is now forced. And I have taken your suggestion of

"\n".join

in the string assertion.

@michaelDCurran michaelDCurran merged commit 468761f into master Sep 16, 2020
@michaelDCurran michaelDCurran deleted the listItemAsMarker branch September 16, 2020 10:46
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.

4 participants