Skip to content

CaretPosition should be EnabledBySetting#41567

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
nt1m:eng/CaretPosition-should-be-EnabledBySetting
Feb 28, 2025
Merged

CaretPosition should be EnabledBySetting#41567
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
nt1m:eng/CaretPosition-should-be-EnabledBySetting

Conversation

@nt1m

@nt1m nt1m commented Feb 27, 2025 •
edited by webkit-early-warning-system
Loading

Copy link
Copy Markdown
Member

c4e24f8

CaretPosition should be EnabledBySetting
https://bugs.webkit.org/show_bug.cgi?id=288644
rdar://145688017

Reviewed by Simon Fraser.

https://commits.webkit.org/287047@main (bug 172137) added Document.caretPositionFromPoint().

However, while Document.caretPositionFromPoint() itself has [EnabledBySetting=CaretPositionFromPointEnabled] neither CaretPosition nor CaretPositionFromPointOptions do.
These should also be behind the flag, because otherwise we break anyone doing anything like `if (window.CaretPosition)` or the mdn-bcd-collector
trying to fetch meaningful data about what browser supports the related APIs.

* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
* Source/WebCore/dom/CaretPosition.idl:

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

ebfb1f4

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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@nt1m nt1m requested review from cdumez and rniwa as code owners February 27, 2025 23:47
@nt1m nt1m self-assigned this Feb 27, 2025
@nt1m nt1m added the CSS Cascading Style Sheets implementation label Feb 27, 2025
@webkit-early-warning-system

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

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash dc29742)

Details

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

@nt1m nt1m added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Feb 27, 2025
@nt1m nt1m force-pushed the eng/CaretPosition-should-be-EnabledBySetting branch from dc29742 to ebfb1f4 Compare February 28, 2025 00:54
@webkit-early-warning-system

webkit-early-warning-system commented Feb 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

Details

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
✅ 🛠 🧪 unsafe-merge ✅ 🛠 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 Feb 28, 2025
@annevk annevk removed the merging-blocked Applied to prevent a change from being merged label Feb 28, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Feb 28, 2025
@webkit-ews-buildbot

webkit-ews-buildbot commented Feb 28, 2025

Copy link
Copy Markdown
Collaborator

Failed gtk-wk2, wpe-wk2 checks. Please resolve failures and re-apply safe-merge-queue label.

Rejecting #41567 from merge queue.

@webkit-ews-buildbot webkit-ews-buildbot removed the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Feb 28, 2025
@webkit-ews-buildbot

webkit-ews-buildbot commented Feb 28, 2025

Copy link
Copy Markdown
Collaborator

Safe-Merge-Queue: Build #50082.

@nt1m nt1m added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged labels Feb 28, 2025
@nt1m
CaretPosition should be EnabledBySetting
c4e24f8 
https://bugs.webkit.org/show_bug.cgi?id=288644
rdar://145688017

Reviewed by Simon Fraser.

https://commits.webkit.org/287047@main (bug 172137) added Document.caretPositionFromPoint().

However, while Document.caretPositionFromPoint() itself has [EnabledBySetting=CaretPositionFromPointEnabled] neither CaretPosition nor CaretPositionFromPointOptions do.
These should also be behind the flag, because otherwise we break anyone doing anything like `if (window.CaretPosition)` or the mdn-bcd-collector
trying to fetch meaningful data about what browser supports the related APIs.

* Source/WebCore/bindings/js/WebCoreBuiltinNames.h:
* Source/WebCore/dom/CaretPosition.idl:

Canonical link: https://commits.webkit.org/291314@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/CaretPosition-should-be-EnabledBySetting branch from ebfb1f4 to c4e24f8 Compare February 28, 2025 12:31
@webkit-commit-queue

webkit-commit-queue commented Feb 28, 2025

Copy link
Copy Markdown
Collaborator

Committed 291314@main (c4e24f8): https://commits.webkit.org/291314@main

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

@webkit-commit-queue webkit-commit-queue merged commit c4e24f8 into WebKit:main Feb 28, 2025
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 28, 2025
@nt1m nt1m deleted the eng/CaretPosition-should-be-EnabledBySetting branch February 28, 2025 13:11
@caugner caugner mentioned this pull request May 9, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smfr smfr smfr approved these changes

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

Assignees

@nt1m nt1m

Labels

CSS Cascading Style Sheets implementation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@nt1m @webkit-early-warning-system @webkit-ews-buildbot @webkit-commit-queue @smfr @annevk