Skip to content

Report destination of link in Word / Outlook legacy #17292

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

Merged
merged 12 commits into from
Nov 4, 2024

Conversation

CyrilleB79
Copy link
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
Copy link
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)

@CyrilleB79 CyrilleB79 marked this pull request as draft October 20, 2024 21:22
@AppVeyorBot
Copy link

  • 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
Copy link

  • 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
Copy link

  • 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 merged commit 10e1c43 into nvaccess:master Nov 4, 2024
4 of 5 checks passed
@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
seanbudd pushed a commit that referenced this pull request Dec 11, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants