When reporting URL of a link use caret position instead of navigator object #14723

lukaszgo1 · 2023-03-16T15:39:36Z

Link to issue number:

Summary of the issue:

In #14583 @XLTechie added a global commands which allows to report a URL for the link at the position of the navigator object. While this is a huge improvement compared to either having copy the URL to the clipboard or accessing the URL from the status bar from the web browser, the fact that these scripts report destination for the link at the navigator object rather than focus / caret position makes their usage impractical for people who prefer to work with review cursor / navigator object not following focus / caret.

Description of user facing changes

When user requests URL of a link NVDA reports URL of a link at the position of the caret / focus.

Description of development approach

Script for reporting URL was modified so that it first tries to get an object at the current caret position, and if that fails from the focused object. User guide and documentation for global commands were updated accordingly.
After @XLTechie's review it was noticed that the script fails to work for links without href attribute, as part of this PR I've added handling for such links.

Testing strategy:

Tested various objects which are links, linked graphics, and non links in both focus and browse mode, in Firefox and IE.

Known issues with pull request:

None known

Change log entries:

Changes

Script for reporting destination of a link now reports destination for the link at the caret / focus position rather than for the navigator object.

Bug fixes:

When trying to report URL for links without href attribute NVDA is no longer silent - the fact that the given link has no destination is reported instead.

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.

…object

lukaszgo1 · 2023-03-16T15:40:12Z

@XLTechie You may want to review / test this.

XLTechie

I think this will be fine. I'm not really sure if it covers more common use cases, but it appears to be slightly more flexible, at the cost of making the review cursor a bit less useful for link identification with some configurations. That probably won't be a problem for most users, and as stated, most likely won't notice either way.

source/globalCommands.py

user_docs/en/userGuide.t2t

XLTechie · 2023-03-23T08:51:40Z

@lukaszgo1 As an aside, you may want to handle the case of the very first link/graphic (Ctrl+Home position) on the Appveyor website. Noticed when downloading the try build for this PR in Firefox.

"Not a link" is not announced, yet there is no link.

Perhaps if Value is None, we should also announce "not a link"? I would like to say "Link has no apparent destination", because it might still be clickable somehow.

If you try to open the Window version of reportLinkDestination, this happens:

IO - inputCore.InputManager.executeGesture (04:47:09.187) - winInputHook (20220):
Input: kb(desktop):NVDA+k
IO - inputCore.InputManager.executeGesture (04:47:09.325) - winInputHook (20220):
Input: kb(desktop):NVDA+k
ERROR - scriptHandler.executeScript (04:47:09.340) - MainThread (19360):
error executing script: <bound method GlobalCommands.script_reportLinkDestination of <globalCommands.GlobalCommands object at 0x0719EB90>> with gesture 'NVDA+k'
Traceback (most recent call last):
  File "scriptHandler.pyc", line 289, in executeScript
  File "globalCommands.pyc", line 3659, in script_reportLinkDestination
  File "ui.pyc", line 106, in browseableMessage
  File "html\__init__.pyc", line 19, in escape
AttributeError: 'NoneType' object has no attribute 'replace'

If you just try the speaking report, you get the following (dev info included):

IO - speech.speech.speak (03:30:59.874) - MainThread (2428):
Speaking ['heading', 'level 1', 'link', 'AppVeyor']
IO - inputCore.InputManager.executeGesture (03:31:01.816) - winInputHook (18724):
Input: kb(desktop):NVDA+k
IO - inputCore.InputManager.executeGesture (03:31:03.248) - winInputHook (18724):
Input: kb(desktop):NVDA+f1
DEBUG - gui.contextHelp.bindHelpEvent (03:31:03.279) - MainThread (2428):
Did context help binding for LogViewer
INFO - globalCommands.script_navigatorObject_devInfo (03:31:03.334) - MainThread (2428):
Developer info for navigator object:
name: 'AppVeyor'
role: Role.LINK
processID: 11596
roleText: None
states: State.LINKED
isFocusable: False
hasFocus: False
Python object: <NVDAObjects.IAccessible.mozilla.Mozilla object at 0x04B1A970>
Python class mro: (<class 'NVDAObjects.IAccessible.mozilla.Mozilla'>, <class 'NVDAObjects.IAccessible.ia2Web.Ia2Web'>, <class 'NVDAObjects.IAccessible.IAccessible'>, <class 'NVDAObjects.window.Window'>, <class 'NVDAObjects.NVDAObject'>, <class 'documentBase.TextContainerObject'>, <class 'baseObject.ScriptableObject'>, <class 'baseObject.AutoPropertyObject'>, <class 'garbageHandler.TrackedObject'>, <class 'object'>)
description: ''
location: RectLTWH(left=370, top=113, width=144, height=54)
value: None
TextInfo: <class 'NVDAObjects.IAccessible.IA2TextTextInfo'>
appModule: AppModule(appModuleHandler, appName='firefox', processID=11596)
appModule.productName: 'Firefox'
appModule.productVersion: '111.0'
appModule.helperLocalBindingHandle: c_long(116676168)
windowHandle: 2426720
windowClassName: 'MozillaWindowClass'
windowControlID: 0
windowStyle: 382664704
extendedWindowStyle: 256
windowThreadID: 11600
windowText: 'nvda pr14723-27847,9913f99d - AppVeyor — Mozilla Firefox'
displayText: ''
IAccessibleObject: <POINTER(IAccessible2) ptr=0x6fa349c at 8ee030>
IAccessibleChildID: 0
IAccessible event parameters: windowHandle=2426720, objectID=-4, childID=-159383565
IAccessible accName: 'AppVeyor'
IAccessible accRole: ROLE_SYSTEM_LINK
IAccessible accState: STATE_SYSTEM_LINKED, STATE_SYSTEM_VALID (4194304)
IAccessible accDescription: ''
IAccessible accValue: None
IAccessible2 windowHandle: 2426720
IAccessible2 uniqueID: -159383565
IAccessible2 role: ROLE_SYSTEM_LINK
IAccessible2 states: IA2_STATE_OPAQUE, IA2_STATE_SELECTABLE_TEXT (5120)
IAccessible2 attributes: 'display:block;class:collapse-only;margin-bottom:0px;tag:a;text-indent:-9999px;formatting:block;text-align:left;margin-right:0px;margin-left:0px;margin-top:0px;'
IAccessible2 relations: containingDocument * 1, containingTabPane * 1, containingApplication * 1

lukaszgo1 · 2023-03-25T14:11:40Z

[...] I'm not really sure if it covers more common use cases, but it appears to be slightly more flexible, at the cost of making the review cursor a bit less useful for link identification with some configurations.

The only hypothetical case I can think of would be a link which is not reachable in browse mode, and is not focusable. I haven't seen such a control anywhere, so I believe this case can be safely ignored.

As an aside, you may want to handle the case of the very first link/graphic (Ctrl+Home position) on the Appveyor website. Noticed when downloading the try build for this PR in Firefox.
"Not a link" is not announced, yet there is no link.
Perhaps if Value is None, we should also announce "not a link"? I would like to say "Link has no apparent destination", because it might still be clickable somehow.

I've implemented your suggestion, since apparently links with no href are valid in HTML.

AppVeyorBot · 2023-03-25T14:52:50Z

FAIL: Translation comments check. Translation comments missing or unexpectedly included. See build log for more information.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests (tags: installer NVDA). See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/06qi34pq19it3dtf/artifacts/output/nvda_snapshot_pr14723-27900,089b35eb.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.8,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 22.4,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 21.4,
FINISH_END 0.2

See test results for failed build of commit 089b35eba6

seanbudd

Generally LGTM

source/globalCommands.py

Qchristensen

Would the user guide wording be more correct as "speaks the destination URL of the link at the caret position or current focus" (as it tries the caret position first?)

AppVeyorBot · 2023-03-28T11:15:00Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests (tags: installer NVDA). See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/sbf1750r79jfttpk/artifacts/output/nvda_snapshot_pr14723-27933,8aafd79a.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.8,
INSTALL_END 0.9,
BUILD_START 0.0,
BUILD_END 25.1,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 22.3,
FINISH_END 0.1

See test results for failed build of commit 8aafd79a6c

AppVeyorBot · 2023-03-28T13:43:58Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests (tags: installer NVDA). See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/p45aaestnmoqrbhn/artifacts/output/nvda_snapshot_pr14723-27937,12ca9ea0.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 1.0,
INSTALL_END 1.2,
BUILD_START 0.0,
BUILD_END 28.0,
TESTSETUP_START 0.0,
TESTSETUP_END 0.4,
TEST_START 0.0,
TEST_END 23.4,
FINISH_END 0.1

See test results for failed build of commit 12ca9ea044

amirsol81 · 2023-03-31T09:51:11Z

@lukaszgo1 @seanbudd @XLTechie The "Report Destination URL" feature seems to be rather inaccurate or problematic at the moment. I'm using the latest alpha release )27981), and NVDA+K, when pressed on some links, just says "Blank".
For instance:

Open the following page: https://www.gsmarena.com/samsung_galaxy_s23_s23_plus_s23_ultra_camera_improvements_march_update-news-58071.php
There are some graphical links on the page such as "Samsung Galaxy S23 review", "Samsung Galaxy S23+ review", "Samsung Galaxy S23 series' March update changelog (machine translated", "Our camera shootout video featuring Galaxy S23 Ultra, vivo X90 Pro, Xiaomi 12S Ultra and 13 Pro is out", etc. G and SHIFT+G are also capable of catching them.
When NVDA+K is pressed on them, the message "Blank" is announced.
Almost all of them point to other pages, and just one of them refers to the same page. I think the destination should be properly caught and reported.

amirsol81 · 2023-04-01T12:52:41Z

@lukaszgo1 @XLTechie @seanbudd Should I create an issue for the bug I reported in my previous comment, or is the comment enough?

lukaszgo1 · 2023-04-01T13:55:08Z

@amirsol81 I cannot reproduce this in latest Firefox Nightly. Could you please create a new issue. In the bug report please mention what web browser you';re using, and if this is a regression from this PR, i.e. if this worked when the URL was reported for the navigator object.

seanbudd · 2023-04-03T00:35:34Z

yes, please create an issue. Comments on closed PRs are easily lost.

amirsol81 · 2023-04-03T09:02:01Z

@seanbudd Thanks - I will.

When reporting URL of a link use caret position instead of navigator …

4fa3aa9

…object

lukaszgo1 requested review from a team as code owners March 16, 2023 15:39

lukaszgo1 requested review from Qchristensen and seanbudd March 16, 2023 15:39

This comment was marked as outdated.

Sign in to view

XLTechie suggested changes Mar 23, 2023

View reviewed changes

source/globalCommands.py Outdated Show resolved Hide resolved

source/globalCommands.py Outdated Show resolved Hide resolved

user_docs/en/userGuide.t2t Outdated Show resolved Hide resolved

lukaszgo1 added 3 commits March 25, 2023 13:52

Merge branch 'master' into i14659

7f622d0

Handle links with no href

fbdb324

Improve description of the modifed scripts.

8827af2

Move comment for translators

f746b13

seanbudd approved these changes Mar 28, 2023

View reviewed changes

source/globalCommands.py Outdated Show resolved Hide resolved

Qchristensen requested changes Mar 28, 2023

View reviewed changes

lukaszgo1 added 2 commits March 28, 2023 12:05

Merge branch 'master' into i14659

6f7c6be

Rework script documentation and user guide

9f69eb3

lukaszgo1 requested a review from Qchristensen March 28, 2023 10:25

Bump for CI

3ed7d10

Second bump for CI

fd865f0

seanbudd approved these changes Mar 28, 2023

View reviewed changes

update changes

414272e

seanbudd merged commit 57bf2a3 into nvaccess:master Mar 29, 2023

nvaccessAuto added this to the 2023.2 milestone Mar 29, 2023

lukaszgo1 deleted the i14659 branch March 29, 2023 09:39

lukaszgo1 mentioned this pull request Apr 3, 2023

Chrome/Edge: NVDA unable to report the destination of various link types #14779

Open

CyrilleB79 mentioned this pull request Dec 10, 2024

PowerPoint: Report link destination #17435

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

When reporting URL of a link use caret position instead of navigator object #14723

When reporting URL of a link use caret position instead of navigator object #14723

lukaszgo1 commented Mar 16, 2023 •

edited

Loading

lukaszgo1 commented Mar 16, 2023

This comment was marked as outdated.

XLTechie left a comment

XLTechie commented Mar 23, 2023

lukaszgo1 commented Mar 25, 2023

AppVeyorBot commented Mar 25, 2023

seanbudd left a comment

Qchristensen left a comment

AppVeyorBot commented Mar 28, 2023

AppVeyorBot commented Mar 28, 2023

amirsol81 commented Mar 31, 2023

amirsol81 commented Apr 1, 2023

lukaszgo1 commented Apr 1, 2023

seanbudd commented Apr 3, 2023

amirsol81 commented Apr 3, 2023

When reporting URL of a link use caret position instead of navigator object #14723

When reporting URL of a link use caret position instead of navigator object #14723

Conversation

lukaszgo1 commented Mar 16, 2023 • edited Loading

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

lukaszgo1 commented Mar 16, 2023

This comment was marked as outdated.

XLTechie left a comment

Choose a reason for hiding this comment

XLTechie commented Mar 23, 2023

lukaszgo1 commented Mar 25, 2023

AppVeyorBot commented Mar 25, 2023

seanbudd left a comment

Choose a reason for hiding this comment

Qchristensen left a comment

Choose a reason for hiding this comment

AppVeyorBot commented Mar 28, 2023

AppVeyorBot commented Mar 28, 2023

amirsol81 commented Mar 31, 2023

amirsol81 commented Apr 1, 2023

lukaszgo1 commented Apr 1, 2023

seanbudd commented Apr 3, 2023

amirsol81 commented Apr 3, 2023

lukaszgo1 commented Mar 16, 2023 •

edited

Loading