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

The textarea element URL encodes pasteboard data #18902

Merged
merged 1 commit into from
Oct 11, 2023

Conversation

rr-codes
Copy link
Contributor

@rr-codes rr-codes commented Oct 10, 2023

a222530

The textarea element URL encodes pasteboard data
https://bugs.webkit.org/show_bug.cgi?id=261936
rdar://116056298

Reviewed by Wenson Hsieh.

In iOS 17 and aligned OS versions, `+[NSURL URLWithString:]` now percent-encodes invalid characters
to help create a valid URL, where previously it would return `nil`.

As a result of this new behavior, strings are now made representable as URLs more easily, and so
their URL representations are added to the pasteboard with percent-encoded invalid characters.

Fix by using the new `+[NSURL URLWithString:encodingInvalidCharacters:]` method, which restores
pre-iOS 17 behavior.

* Source/WTF/wtf/PlatformHave.h:
* Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:
(WebCore::addRepresentationsForPlainText):
* Tools/TestWebKitAPI/Tests/ios/UIPasteboardTests.mm:
(TestWebKitAPI::TEST):

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

7626c09

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

@rr-codes rr-codes self-assigned this Oct 10, 2023
@rr-codes rr-codes added the Forms For bugs specific to form elements (checkboxes, buttons, text fields, etc.) label Oct 10, 2023
@aproskuryakov
Copy link
Contributor

Why do we try to create a URL when copying text from an input field?

|| PLATFORM(VISION) \
|| (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 100000) \
|| (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 170000)
#define HAVE_NEW_NSURL_PARSING 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Using "new" in feature names is an anti-pattern. It will be old in no time, with a very misleading name.

Something like HAVE_NSURL_ENCODING_INVALID_CHARACTERS would seem better.

@rr-codes
Copy link
Contributor Author

Why do we try to create a URL when copying text from an input field?

When copying any text, we try to see if it is a valid URL and if so, copy the URL representation as well as the plain text representation to the pasteboard, so that it can be pasted as either of those two types.

@rr-codes rr-codes added the merge-queue Applied to send a pull request to merge-queue label Oct 10, 2023
@aproskuryakov
Copy link
Contributor

I think that this restates what we do, but does not explain why. Would anything break if we just removed this code?

@rr-codes
Copy link
Contributor Author

I think that this restates what we do, but does not explain why. Would anything break if we just removed this code?

Yes; pasting it into Notes for example would make it be a hyperlink currently, but wouldn't do so if I removed the code

@aproskuryakov
Copy link
Contributor

It's not clear to me if automatically linkifying test from <textarea> is correct behavior. But I guess the more mysterious it is, the harder it is to change.

https://bugs.webkit.org/show_bug.cgi?id=261936
rdar://116056298

Reviewed by Wenson Hsieh.

In iOS 17 and aligned OS versions, `+[NSURL URLWithString:]` now percent-encodes invalid characters
to help create a valid URL, where previously it would return `nil`.

As a result of this new behavior, strings are now made representable as URLs more easily, and so
their URL representations are added to the pasteboard with percent-encoded invalid characters.

Fix by using the new `+[NSURL URLWithString:encodingInvalidCharacters:]` method, which restores
pre-iOS 17 behavior.

* Source/WTF/wtf/PlatformHave.h:
* Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:
(WebCore::addRepresentationsForPlainText):
* Tools/TestWebKitAPI/Tests/ios/UIPasteboardTests.mm:
(TestWebKitAPI::TEST):

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

Committed 269178@main (a222530): https://commits.webkit.org/269178@main

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

@webkit-commit-queue webkit-commit-queue merged commit a222530 into WebKit:main Oct 11, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Forms For bugs specific to form elements (checkboxes, buttons, text fields, etc.)
Projects
None yet
6 participants