Skip to content
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

Add a feature flag to allow or disallow writing RTF to the pasteboard when copying/dragging #20880

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented Nov 24, 2023

ccdf275

Add a feature flag to allow or disallow writing RTF to the pasteboard when copying/dragging
https://bugs.webkit.org/show_bug.cgi?id=265323
rdar://118419812

Reviewed by Richard Robinson.

On some Cocoa OS versions, the pasteboard will (if necessary) automatically convert HTML and web
archive data written to the pasteboard to RTF, RTFD, or archived `NSAttributedString` data when
pasting in an app that only consumes these native rich text formats.

When this behavior is available (guarded by `HAVE(PASTEBOARD_CONVERSION_FROM_HTML_TO_RTF)`), WebKit
no longer needs to eagerly write RTF, RTFD or attributed string representations of the selected text
to the pasteboard when copying or dragging text, since we can defer to system support for performing
this conversion on paste/drop.

Make some minor adjustments to pasteboard writing logic, to make the attributed string writing part
conditional on this new setting.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/editing/Editor.h:
* Source/WebCore/editing/cocoa/EditorCocoa.mm:
(WebCore::populateRichTextDataIfNeeded):
(WebCore::Editor::writeSelectionToPasteboard):
(WebCore::Editor::writeSelection):

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

984fef7

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
βœ… πŸ§ͺ ios-wk2-wpt βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  πŸ§ͺ jsc-arm64 βœ… πŸ›  tv βœ… πŸ§ͺ mac-AS-debug-wk2 ❌ πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim βœ… πŸ›  jsc-armv7
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  watch-sim

@whsieh whsieh requested a review from rniwa as a code owner November 24, 2023 19:38
@whsieh whsieh self-assigned this Nov 24, 2023
@whsieh whsieh added the HTML Editing For bugs in HTML editing support (including designMode and contentEditable). label Nov 24, 2023
webContent.dataInRTFDFormat = [string containsAttachments] ? dataInRTFDFormat(string.get()) : nullptr;
webContent.dataInRTFFormat = dataInRTFFormat(string.get());
webContent.dataInAttributedStringFormat = archivedDataForAttributedString(string.get());
if (document->settings().writeRichTextDataWhenCopyingOrDragging()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this could probably be refactored into a generic function since it’s the same few lines as above, but that’s probably overkill.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not too unwieldy, though I'd need to pull the static dataInRTFDFormat / dataInRTFFormat helpers out into the public section so that I can use them here:

template<typename PasteboardContent>
void populateRichTextDataIfNeeded(PasteboardContent& content, const Document& document)
{
    if (!document.settings().writeRichTextDataWhenCopyingOrDragging())
        return;

    auto string = selectionAsAttributedString(document);
    content.dataInRTFDFormat = [string containsAttachments] ? Editor::dataInRTFDFormat(string.get()) : nullptr;
    content.dataInRTFFormat = Editor::dataInRTFFormat(string.get());
    content.dataInAttributedStringFormat = archivedDataForAttributedString(string.get());
}

Thanks for the review!

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 24, 2023
@whsieh whsieh removed the merging-blocked Applied to prevent a change from being merged label Nov 24, 2023
@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label Nov 24, 2023
… when copying/dragging

https://bugs.webkit.org/show_bug.cgi?id=265323
rdar://118419812

Reviewed by Richard Robinson.

On some Cocoa OS versions, the pasteboard will (if necessary) automatically convert HTML and web
archive data written to the pasteboard to RTF, RTFD, or archived `NSAttributedString` data when
pasting in an app that only consumes these native rich text formats.

When this behavior is available (guarded by `HAVE(PASTEBOARD_CONVERSION_FROM_HTML_TO_RTF)`), WebKit
no longer needs to eagerly write RTF, RTFD or attributed string representations of the selected text
to the pasteboard when copying or dragging text, since we can defer to system support for performing
this conversion on paste/drop.

Make some minor adjustments to pasteboard writing logic, to make the attributed string writing part
conditional on this new setting.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:
* Source/WebCore/editing/Editor.h:
* Source/WebCore/editing/cocoa/EditorCocoa.mm:
(WebCore::populateRichTextDataIfNeeded):
(WebCore::Editor::writeSelectionToPasteboard):
(WebCore::Editor::writeSelection):

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

Committed 271106@main (ccdf275): https://commits.webkit.org/271106@main

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

@webkit-commit-queue webkit-commit-queue merged commit ccdf275 into WebKit:main Nov 24, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 24, 2023
@whsieh whsieh deleted the eng/265323 branch November 25, 2023 05:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTML Editing For bugs in HTML editing support (including designMode and contentEditable).
Projects
None yet
5 participants