Skip to content

AX: Downgrade various text marker asserts and logs to debug-only #47300

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 27, 2025
Merged

AX: Downgrade various text marker asserts and logs to debug-only #47300

merged 1 commit into from
Jun 27, 2025
+29 −43

Conversation

twilco
Copy link
Contributor

@twilco twilco commented Jun 27, 2025 •
edited by webkit-early-warning-system
Loading

e5d6551

AX: Downgrade various text marker asserts and logs to debug-only
https://bugs.webkit.org/show_bug.cgi?id=295102
rdar://154493503

Reviewed by Joshua Hoffman.

Making these asserts and logs run in release mode was useful when developing the feature, but we shouldn't continue
paying the cost.

* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::textMarkerDataForVisiblePosition):
* Source/WebCore/accessibility/AXTextMarker.cpp:
(WebCore::TextMarkerData::TextMarkerData):
(WebCore::AXTextMarker::convertToDomOffset const):
(WebCore::AXTextMarker::offsetFromRoot const):
(WebCore::AXTextMarker::nextMarkerFromOffset const):
(WebCore::AXTextMarker::findLastBefore const):
(WebCore::AXTextMarker::rangeWithSameStyle const):
(WebCore::AXTextMarkerRange::viewportRelativeFrame const):
(WebCore::AXTextMarkerRange::convertToDomOffsetRange const):
* Source/WebCore/accessibility/AXTextMarker.h:
* Source/WebCore/accessibility/AXTextRun.cpp:
(WebCore::AXTextRuns::localRect const):
* Source/WebCore/accessibility/AXTextRun.h:
(WebCore::AXTextRun::AXTextRun):
* Source/WebCore/accessibility/cocoa/AXTextMarkerCocoa.mm:
(WebCore::AXTextMarkerRange::toAttributedString const):

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

e49b124

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

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

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

EWS run on previous version of this PR (hash 15267d2)

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
✅ 🧪 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 27, 2025
@twilco twilco force-pushed the eng/AX-Downgrade-various-text-marker-asserts-and-logs-to-debug-only branch from 15267d2 to 712f8fb Compare June 27, 2025 16:01
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on previous version of this PR (hash 712f8fb)

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
✅ 🧪 vision-wk2   🧪 mac-intel-wk2 ⏳ 🛠 playstation
✅ 🛠 tv ⏳ 🛠 mac-safer-cpp
⏳ 🛠 tv-sim
⏳ 🛠 watch
⏳ 🛠 watch-sim

@twilco twilco force-pushed the eng/AX-Downgrade-various-text-marker-asserts-and-logs-to-debug-only branch from 712f8fb to e49b124 Compare June 27, 2025 16:10
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on current version of this PR (hash e49b124)

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

@twilco twilco 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 27, 2025
@webkit-commit-queue webkit-commit-queue force-pushed the eng/AX-Downgrade-various-text-marker-asserts-and-logs-to-debug-only branch from e49b124 to 45c24e1 Compare June 27, 2025 19:48
@twilco
AX: Downgrade various text marker asserts and logs to debug-only
e5d6551 
https://bugs.webkit.org/show_bug.cgi?id=295102
rdar://154493503

Reviewed by Joshua Hoffman.

Making these asserts and logs run in release mode was useful when developing the feature, but we shouldn't continue
paying the cost.

* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::textMarkerDataForVisiblePosition):
* Source/WebCore/accessibility/AXTextMarker.cpp:
(WebCore::TextMarkerData::TextMarkerData):
(WebCore::AXTextMarker::convertToDomOffset const):
(WebCore::AXTextMarker::offsetFromRoot const):
(WebCore::AXTextMarker::nextMarkerFromOffset const):
(WebCore::AXTextMarker::findLastBefore const):
(WebCore::AXTextMarker::rangeWithSameStyle const):
(WebCore::AXTextMarkerRange::viewportRelativeFrame const):
(WebCore::AXTextMarkerRange::convertToDomOffsetRange const):
* Source/WebCore/accessibility/AXTextMarker.h:
* Source/WebCore/accessibility/AXTextRun.cpp:
(WebCore::AXTextRuns::localRect const):
* Source/WebCore/accessibility/AXTextRun.h:
(WebCore::AXTextRun::AXTextRun):
* Source/WebCore/accessibility/cocoa/AXTextMarkerCocoa.mm:
(WebCore::AXTextMarkerRange::toAttributedString const):

Canonical link: https://commits.webkit.org/296747@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/AX-Downgrade-various-text-marker-asserts-and-logs-to-debug-only branch from 45c24e1 to e5d6551 Compare June 27, 2025 19:50
@webkit-commit-queue
Copy link
Collaborator

Committed 296747@main (e5d6551): https://commits.webkit.org/296747@main

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

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

@hoffmanjoshua hoffmanjoshua hoffmanjoshua approved these changes

Assignees

@twilco twilco

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
@twilco @webkit-early-warning-system @webkit-commit-queue @hoffmanjoshua @webkit-ews-buildbot