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

Add support of getting link destination of graphic links #14783

Merged
merged 3 commits into from Apr 5, 2023

Conversation

beqabeqa473
Copy link
Contributor

@beqabeqa473 beqabeqa473 commented Apr 4, 2023

Link to issue number:

Closes #14779

Summary of the issue:

destination of graphic links are not reported in chrome/edge

Description of user facing changes

Destination of graphic links now should be reported in chrome/edge browsers

Description of development approach

NVDA lands its focus to graphic elemeng "img" which does not have href value, but an element which is parent of that element is link.
So one more check was introduced to ensure han if we are focusing a graphic element and its parent is a link, we are now replacing an object by its parent object

Testing strategy:

Opened a webpage with graphic links and ensured that their addresses are correctly reported

Known issues with pull request:

None

Change log entries:

New features
Changes
Bug fixes
Destination of graphic links are now corectly reported in chrome and edge browsers
For Developers

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@beqabeqa473 beqabeqa473 requested a review from a team as a code owner April 4, 2023 06:22
@Adriani90
Copy link
Collaborator

Are image links also covered? See
https://www.rapidtables.com/web/html/link/html-image-link.html

@beqabeqa473
Copy link
Contributor Author

That was a purpose of creating this pr

@Adriani90
Copy link
Collaborator

Ah ok sorry I thought there might be a difference between grafic links and image links, didn't mean to spam this. :-)

source/globalCommands.py Outdated Show resolved Hide resolved
source/globalCommands.py Show resolved Hide resolved
@seanbudd
Copy link
Member

seanbudd commented Apr 4, 2023

Thanks @beqabeqa473 - just a minor fix required in regards to comments

@AppVeyorBot
Copy link

See test results for failed build of commit fbca680220

@seanbudd seanbudd merged commit f92ee61 into nvaccess:master Apr 5, 2023
1 check was pending
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Apr 5, 2023
@amirsol81
Copy link

@beqabeqa473 @seanbudd I have Alpha 28043 for this graphic link-oriented fix, but it introduces one more serious issue.

  1. Go to https://www.gsmarena.com/
  2. Look for graphic links - many of them are there.
  3. Having located either of them, press the Down arrow once to move to its following heading which also contains the link in non-graphic form. For instance, the graphic link might be "Xiaomi Redmi Note 12 4G review." Press the Down arrow once to move to the heading which says: "link heading level 3 Xiaomi Redmi Note 12 Pro review."
  4. Press NVDA+K.
    NVDA incorrectly says: "Not a link." These are absolutely valid links which are now being ignored. This is with Chrome 112.

@beqabeqa473
Copy link
Contributor Author

It is not related for this pr and linked issue.
I still have earlier alpha on my machine and can reproduce bug you are talking

@amirsol81
Copy link

amirsol81 commented Apr 6, 2023 via email

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.

Chrome/Edge: NVDA unable to report the destination of various link types
6 participants