Skip to content

AX: string for text marker range should not skip an emitted newline if it's the first character #47224

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2025
Merged

AX: string for text marker range should not skip an emitted newline if it's the first character #47224

merged 1 commit into from
Jun 26, 2025
+102 −2

Conversation

minorninth
Copy link
Contributor

@minorninth minorninth commented Jun 26, 2025 •
edited by webkit-early-warning-system
Loading

bde3bff

AX: string for text marker range should not skip an emitted newline if it's the first character
https://bugs.webkit.org/show_bug.cgi?id=295017
rdar://154368379

Reviewed by Tyler Wilcock.

The user-facing issue was that VoiceOver would fail to speak the text
of a new line in a document when navigating by line using caret
navigation, because VoiceOver thought the new cursor position was
visually equivalent to the previous one.

The underlying cause was that if a text marker range consisted of only
an emitted newline, the newline was being suppressed by mistake, so it
made it seem like the range was empty, indicating that two positions
were equivalent when in fact they differ due to the emitted newline.

* LayoutTests/accessibility/isolated-tree/simple-word-navigation-expected.txt:
* LayoutTests/accessibility/mac/text-marker-emitted-newlines-expected.txt: Added.
* LayoutTests/accessibility/mac/text-marker-emitted-newlines.html: Added.
* Source/WebCore/accessibility/AXTextMarker.cpp:
(WebCore::AXTextMarkerRange::toString const):

Canonical link: https://commits.webkit.org/296690@main

594443a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@minorninth minorninth self-assigned this Jun 26, 2025
@minorninth minorninth added the Accessibility For bugs related to accessibility. label Jun 26, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 26, 2025 •
edited
Loading

EWS run on current version of this PR (hash 594443a)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 26, 2025
@minorninth minorninth added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Jun 26, 2025
@minorninth
AX: string for text marker range should not skip an emitted newline i…
bde3bff 
…f it's the first character

https://bugs.webkit.org/show_bug.cgi?id=295017
rdar://154368379

Reviewed by Tyler Wilcock.

The user-facing issue was that VoiceOver would fail to speak the text
of a new line in a document when navigating by line using caret
navigation, because VoiceOver thought the new cursor position was
visually equivalent to the previous one.

The underlying cause was that if a text marker range consisted of only
an emitted newline, the newline was being suppressed by mistake, so it
made it seem like the range was empty, indicating that two positions
were equivalent when in fact they differ due to the emitted newline.

* LayoutTests/accessibility/isolated-tree/simple-word-navigation-expected.txt:
* LayoutTests/accessibility/mac/text-marker-emitted-newlines-expected.txt: Added.
* LayoutTests/accessibility/mac/text-marker-emitted-newlines.html: Added.
* Source/WebCore/accessibility/AXTextMarker.cpp:
(WebCore::AXTextMarkerRange::toString const):

Canonical link: https://commits.webkit.org/296690@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/AX-string-for-text-marker-range-should-not-skip-an-emitted-newline-if-it-s-the-first-character branch from 594443a to bde3bff Compare June 26, 2025 23:10
@webkit-commit-queue
Copy link
Collaborator

Committed 296690@main (bde3bff): https://commits.webkit.org/296690@main

Reviewed commits have been landed. Closing PR #47224 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit bde3bff into WebKit:main Jun 26, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twilco twilco twilco approved these changes

Assignees

@minorninth minorninth

Labels
Accessibility For bugs related to accessibility.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@minorninth @webkit-early-warning-system @webkit-commit-queue @twilco @webkit-ews-buildbot