Skip to content

Report destination of link in Word / Outlook legacy#17292

Merged
seanbudd merged 12 commits into
nvaccess:masterfrom
CyrilleB79:wordLink
Nov 4, 2024
Merged

Report destination of link in Word / Outlook legacy#17292
seanbudd merged 12 commits into
nvaccess:masterfrom
CyrilleB79:wordLink

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented Oct 14, 2024

Copy link
Copy Markdown
Contributor

Link to issue number:

None

Summary of the issue:

The command to report the destination of a link (NVDA+k) does not work in Word or Outlook when using object model (by default for Windows 10 users or users of older Office versions).

Description of user facing changes

NVDA+k will wow report the destination of the link in Word/Outlook object model.

Description of development approach

  • Created a specific _getLinkDataAtCaretPosition function to report the required information on a link in a WordDocumentTextInfo. Other text info classes can define such functions in the future to support the command in other content types.
  • I have not moved the initial implementation to support link reporting in browsers since I do not know exactly in which text info I should move it to guarantee that I'll not break any already supported use case. I may change move this if requested.

Testing strategy:

Manual tests:

  • Tested the command in Word/Outlook, UIA/legacy, browse mode/focus mode
  • Tested the activate command on links in Word since its code has been modified

Known issues with pull request:

None

Code Review Checklist:

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

@coderabbitai summary

@CyrilleB79 CyrilleB79 marked this pull request as ready for review October 16, 2024 14:54
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner October 16, 2024 14:54
@CyrilleB79 CyrilleB79 requested a review from seanbudd October 16, 2024 14:54
@CyrilleB79

CyrilleB79 commented Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

@seanbudd, I have passed this PR to ready.

Let me know if you wish me to move the original code in suitable text info object(s) rather than to keep it in the global commands class. Moving this code would surely be cleaner regarding the code organization; but I am more likely to break existing use cases. If you push for moving this code, have you an idea where I should move it? (advice appreciated)

seanbudd
seanbudd reviewed Oct 16, 2024
View reviewed changes
Comment thread source/NVDAObjects/window/winword.py Outdated
Comment thread source/NVDAObjects/window/winword.py Outdated
Comment thread source/NVDAObjects/window/winword.py Outdated
Comment thread source/NVDAObjects/window/winword.py Outdated
Comment thread source/treeInterceptorHandler.py Outdated
Comment thread source/textInfos/__init__.py Outdated
seanbudd
seanbudd reviewed Oct 16, 2024
View reviewed changes
Comment thread source/globalCommands.py Outdated
CyrilleB79 and others added 2 commits October 17, 2024 22:24
@CyrilleB79 @seanbudd
Update source/NVDAObjects/window/winword.py
8c906e3 
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@CyrilleB79
Addressing review
2b98b4a
@CyrilleB79 CyrilleB79 marked this pull request as draft October 20, 2024 21:22
@AppVeyorBot

AppVeyorBot commented Oct 20, 2024

Copy link
Copy Markdown
  • PASS: Translation comments check.
  • PASS: License check.
  • PASS: Unit tests.
  • FAIL: Lint check. See test results and lint artifacts for more information.
  • PASS: System tests (tags: installer NVDA).
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/0rkxmy6od5rjxdi3/artifacts/output/l10nUtil.exe nvda_snapshot_pr17292-34359,9167cb9e.exe
  • CI timing (mins):
    INIT 0.0,
    INSTALL_START 1.2,
    INSTALL_END 1.0,
    BUILD_START 0.0,
    BUILD_END 24.1,
    TESTSETUP_START 0.0,
    TESTSETUP_END 0.4,
    TEST_START 0.0,
    TEST_END 18.4,
    FINISH_END 0.2

See test results for failed build of commit 9167cb9e7d

@AppVeyorBot

AppVeyorBot commented Oct 21, 2024

Copy link
Copy Markdown
  • PASS: Translation comments check.
  • PASS: License check.
  • FAIL: Unit tests. See test results for more information.
  • 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/e4u9nlw31qgniviq/artifacts/output/l10nUtil.exe nvda_snapshot_pr17292-34365,c1eafa4a.exe
  • CI timing (mins):
    INIT 0.0,
    INSTALL_START 1.5,
    INSTALL_END 1.0,
    BUILD_START 0.0,
    BUILD_END 28.1,
    TESTSETUP_START 0.0,
    TESTSETUP_END 0.3,
    TEST_START 0.0,
    TEST_END 1.1,
    FINISH_END 0.2

See test results for failed build of commit c1eafa4af3

@AppVeyorBot

AppVeyorBot commented Oct 21, 2024

Copy link
Copy Markdown
  • PASS: Translation comments check.
  • PASS: License 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/xtw723cegtieq5j9/artifacts/output/l10nUtil.exe nvda_snapshot_pr17292-34369,46e38411.exe
  • CI timing (mins):
    INIT 0.0,
    INSTALL_START 1.5,
    INSTALL_END 0.9,
    BUILD_START 0.0,
    BUILD_END 26.5,
    TESTSETUP_START 0.0,
    TESTSETUP_END 0.4,
    TEST_START 0.0,
    TEST_END 18.8,
    FINISH_END 0.2

See test results for failed build of commit 46e38411b8

@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Oct 21, 2024
@CyrilleB79 CyrilleB79 marked this pull request as ready for review October 23, 2024 17:30
seanbudd
seanbudd approved these changes Nov 4, 2024
View reviewed changes
Comment thread source/textInfos/__init__.py Outdated
@seanbudd seanbudd merged commit 10e1c43 into nvaccess:master Nov 4, 2024
@github-actions github-actions Bot added this to the 2025.1 milestone Nov 4, 2024
@CyrilleB79 CyrilleB79 deleted the wordLink branch November 4, 2024 22:14
@CyrilleB79 CyrilleB79 mentioned this pull request Nov 25, 2024
Merged
5 tasks
seanbudd pushed a commit that referenced this pull request Dec 11, 2024
@CyrilleB79
PowerPoint: Report link destination (#17435)
bdd3153 
When in PowerPoint, NVDA+K does not work to report a link.

Description of user facing changes
In PowerPoint slides (edition mode), pressing NVDA+K will now report the link destination. This works in text areas as well as on shapes / charts / images that are links.
Also 2 small fixes:
Restored the Copy and Close buttons of the browseable message of NVDA+K that were removed by mistake in Report destination of link in Word / Outlook legacy #17292
In Windows UI, e.g. settings, pressing NVDA+K on a link does not report anymore that there is no link; it rather reports that the destination cannot be reported.
Description of development approach
Use PowerPoint object model
Changed the global strategy to retrieve a link: first try from text infos; if not supported, try from the focused object.
@nishimotz nishimotz mentioned this pull request Apr 14, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

Assignees

No one assigned

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Milestone

2025.1

Development

Successfully merging this pull request may close these issues.

3 participants

@CyrilleB79 @AppVeyorBot @seanbudd