Skip to content

[Unified Text Replacement] Distinguish plain text and rich text replacements#26229

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
pxlcoder:eng/271351
Mar 21, 2024
Merged

[Unified Text Replacement] Distinguish plain text and rich text replacements#26229
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
pxlcoder:eng/271351

Conversation

@pxlcoder
Copy link
Member

@pxlcoder pxlcoder commented Mar 21, 2024

b50554e

[Unified Text Replacement] Distinguish plain text and rich text replacements
https://bugs.webkit.org/show_bug.cgi?id=271351
rdar://123530706

Reviewed by Wenson Hsieh.

Edit actions should have a different effect depending on whether a plain or rich
text replacement is occuring.

* Source/WebCore/dom/SimpleRange.h:
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/UnifiedTextReplacement.serialization.in:
* Source/WebKit/Shared/WebUnifiedTextReplacementContextData.h:
* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::willBeginTextReplacementSession):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::willBeginTextReplacementSession):
* Source/WebKit/WebProcess/WebPage/Cocoa/UnifiedTextReplacementController.mm:
(WebKit::replaceTextInRange):

Refactor to use `Document` rather than `Frame` so that common code can be shared,
and indirection is reduced.

(WebKit::replaceContentsInRange):

Ditto (WebKit::replaceTextInRange).

(WebKit::UnifiedTextReplacementController::willBeginTextReplacementSession):

Store the replacement type for retrieval when an edit action is performed.

(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveReplacements):
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidUpdateStateForReplacement):

Committed and reverted state changes are now permanent. Do not restore the marker
after it is removed.

(WebKit::UnifiedTextReplacementController::didEndTextReplacementSession):
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveTextWithReplacementRange):
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveEditAction):
(WebKit::UnifiedTextReplacementController::textReplacementSessionPerformEditActionForPlainText):

Add a new path for plain text edit actions. Replacements are performed in reverse
order to avoid recomputing ranges.

(WebKit::UnifiedTextReplacementController::textReplacementSessionPerformEditActionForRichText):

Preserve existing rich text behavior.

* Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
(WebKit::WebPage::willBeginTextReplacementSession):
* Source/WebKit/WebProcess/WebPage/UnifiedTextReplacementController.h:

Add a map of UUIDs to replacement types, to ensure the right method is used when
an edit action is received.

* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

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

c4f78cc

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy 🧪 ios-wk2-wpt 🧪 mac-wk1 ✅ 🛠 wpe-skia
🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 tv 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
🛠 tv-sim 🧪 api-gtk
🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@pxlcoder pxlcoder requested review from cdumez and rniwa as code owners March 21, 2024 06:02
@pxlcoder pxlcoder self-assigned this Mar 21, 2024
@pxlcoder pxlcoder added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Mar 21, 2024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Double checking — oldData.originalText in the context of redo refers to the text after replacement?)

Copy link
Member Author

@pxlcoder pxlcoder Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I also realized I don't need to return a tuple here, since the first value is always the same. Will simplify that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this check if the WeakPtr is null and skip if so? Because we're triggering editing commands and layout in this loop, I suspect it's possible that some of the (previously non-null) markers in this vector could become null during iteration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

@pxlcoder pxlcoder added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 21, 2024
…cements

https://bugs.webkit.org/show_bug.cgi?id=271351
rdar://123530706

Reviewed by Wenson Hsieh.

Edit actions should have a different effect depending on whether a plain or rich
text replacement is occuring.

* Source/WebCore/dom/SimpleRange.h:
* Source/WebKit/Scripts/webkit/messages.py:
(headers_for_type):
* Source/WebKit/Shared/UnifiedTextReplacement.serialization.in:
* Source/WebKit/Shared/WebUnifiedTextReplacementContextData.h:
* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::willBeginTextReplacementSession):
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.h:
* Source/WebKit/UIProcess/mac/WebViewImpl.mm:
(WebKit::WebViewImpl::willBeginTextReplacementSession):
* Source/WebKit/WebProcess/WebPage/Cocoa/UnifiedTextReplacementController.mm:
(WebKit::replaceTextInRange):

Refactor to use `Document` rather than `Frame` so that common code can be shared,
and indirection is reduced.

(WebKit::replaceContentsInRange):

Ditto (WebKit::replaceTextInRange).

(WebKit::UnifiedTextReplacementController::willBeginTextReplacementSession):

Store the replacement type for retrieval when an edit action is performed.

(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveReplacements):
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidUpdateStateForReplacement):

Committed and reverted state changes are now permanent. Do not restore the marker
after it is removed.

(WebKit::UnifiedTextReplacementController::didEndTextReplacementSession):
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveTextWithReplacementRange):
(WebKit::UnifiedTextReplacementController::textReplacementSessionDidReceiveEditAction):
(WebKit::UnifiedTextReplacementController::textReplacementSessionPerformEditActionForPlainText):

Add a new path for plain text edit actions. Replacements are performed in reverse
order to avoid recomputing ranges.

(WebKit::UnifiedTextReplacementController::textReplacementSessionPerformEditActionForRichText):

Preserve existing rich text behavior.

* Source/WebKit/WebProcess/WebPage/Cocoa/WebPageCocoa.mm:
(WebKit::WebPage::willBeginTextReplacementSession):
* Source/WebKit/WebProcess/WebPage/UnifiedTextReplacementController.h:

Add a map of UUIDs to replacement types, to ensure the right method is used when
an edit action is received.

* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

Canonical link: https://commits.webkit.org/276490@main
@webkit-commit-queue
Copy link
Collaborator

Committed 276490@main (b50554e): https://commits.webkit.org/276490@main

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

@webkit-commit-queue webkit-commit-queue merged commit b50554e into WebKit:main Mar 21, 2024
@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 Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants