Don't braille or speak duplicate name / content / description#12888
Don't braille or speak duplicate name / content / description#12888feerrenrut merged 7 commits intomasterfrom
Conversation
These tests pass, showing the current behavior
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Adjust system tests to match new behavior
58c014b to
fea18a5
Compare
|
The Firefox crash should now be resolved, please test the [latest PR build to confirm].(https://ci.appveyor.com/api/buildjobs/n8wrf4lcrm3hqn4v/artifacts/output%2Fnvda_snapshot_pr12888-23854%2C2ea82fef.exe) |
|
I can confirm that Firefox crash is fixed on my end. Also I would like to thank @feerrenrut for this fix - not announcing description when it equals the name has been requested for years and it makes various programs (even outside of the web browser) so more pleasant to use. |
|
Thanks @lukaszgo1 for explanation, I confirm all on my part. |
|
Oh thanks @lukaszgo1, I seem to have mixed up my test cases while working on this. There are a few aspects to this:
I will look at reverting the change in behavior for "Report Object Descriptions". I'll also see if I can add further information to the settings panel / user guide to explain the significance of "object". Although this PR does not fix the specifically reported but, it does seem to be a general improvement. I suggest that we merge this on it's own (after updating the description / test cases). I'll continue the work in a new PR. |
michaelDCurran
left a comment
There was a problem hiding this comment.
I would much prefer it if the commit to "tidy up the tests" was separated into a separate pr. This changes other tests than aria-description and creates a 300+ line diff.
But other than that everything before looks good.
Hmm, on github it doesn't show many changes, but the summary does say 300+ lines changed. I'll investigate. Happy to move it to a separate PR. |
|
I have no idea where the 300 is coming from. My local diff shows 26 lines with a + and 18 with a - |
|
Thanks so much for making this happen 👏 |
|
Fixes #7841 |
Link to issue number:
Related to #12750
Summary of the issue:
When a link has a
titleattribute that matches the content, braille (and speech) is duplicated.An example:
Two links URL encoded:
Description of how this pull request fixes the issue:
Tests have been added for speech, to show current behavior (duplicated name/content/description)
The duplicate speech/braille is fixed:
Testing strategy:
Manual testing using the braille viewer with the same sample.
Known issues with pull request:
None
Change log entries:
Bug fixes
Code Review Checklist: