Skip to content

Commit

Permalink
Cherry-pick 81896aa. rdar://122391840
Browse files Browse the repository at this point in the history
    [iOS 17.4] Copying a relative URL in Reader mode and pasting in Messages inserts a safari-reader:// URL
    https://bugs.webkit.org/show_bug.cgi?id=269143
    rdar://122391840

    Reviewed by Ryosuke Niwa.

    Starting in iOS 17.4, Safari no longer writes `NSAttributedString` data directly to the pasteboard.
    Instead, we just write web archive data and HTML, which the system converts (on paste) into an
    `NSAttributedString` only if needed, using UIFoundation (…which uses an offscreen WKWebView under
    the hood, via `nsattributedstringagent`).

    However, when copying a selected link with a relative HREF like `<a href="/foo.html">…</a>` in
    Safari reader, the reader document has a `base` element with an `href` referencing the original
    website URL, while the real document URL has a scheme of `safari-reader://…`. When writing web
    archive data to the pasteboard in Safari, we just write the markup `<a href="/foo.html">…</a>` as-
    is; when converting to an attributed string (e.g., when pasting in Messages) we complete the
    relative URL using the top document URL taken from the web archive, which still uses a scheme of
    `safari-reader://`, so resulting attributed string has a link attribute pointing to a
    `safari-reader://` URL instead of the HTTP URL.

    To avoid this, we detect the fact that there’s a `base` element in the document when creating web
    archive data from the current selection, and prepend the `base` element to the markup, under the
    `head`; in doing so, we ensure that relative URLs will be completed using the same `base` element
    that was present in the document when copying.

    Test: CopyHTML.SanitizationPreservesRelativeURLInAttributedString

    * LayoutTests/platform/ios/editing/pasteboard/onpaste-text-html-expected.txt:
    * LayoutTests/platform/mac/editing/pasteboard/onpaste-text-html-expected.txt:
    * LayoutTests/platform/mac/fast/events/ondrop-text-html-expected.txt:

    Rebaseline layout tests to account for the fact that the `meta` tag is now also inserted below the
    `head` element (as [noted in the spec](https://html.spec.whatwg.org/multipage/semantics.html#the-meta-element)).

    * Source/WebCore/editing/markup.cpp:

    Rename `prependMetaCharsetUTF8TagIfNonASCIICharactersArePresent` to `prependHeadIfNecessary`, and
    make it append both, either or none of the `meta` and `base` elements underneath a `head` element.
    Like before, we only append `meta charset=UTF-8` on Cocoa in the case where the text contains non-
    ASCII characters, for compatibility with system NSPasteboard/UIPasteboard text encoding treatment.
    If the given `baseElement` is non-null, we'll additionally append the base element (along with its
    attributes) to the `head`. Note that if neither of the above elements are needed, we'll just skip
    appending a `head`.

    (WebCore::serializePreservingVisualAppearanceInternal):

    Add support for a new `PreserveBaseElement` option, which indicates whether or not we should attempt
    to preserve the base element, if it exists in the document containing the given range. This is `No`
    by default, and set to `Yes` only when generating web archive data from a selected range in the
    document to ensure that any completed relative URLs in the selected range point to the same resource
    in the web archive, as they previously did in the original document.

    (WebCore::serializePreservingVisualAppearance):
    (WebCore::sanitizedMarkupForFragmentInDocument):
    * Source/WebCore/editing/markup.h:
    * Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp:
    (WebCore::LegacyWebArchive::createFromSelection):

    Pass in `PreserveBaseElement::Yes`. See above for more details.

    * Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig:

    Drive-by fix: remove the redundant `WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS` setting, which was only
    used to link against `UniformTypeIdentifiers` on iOS (but is now unnecessary, since we already
    unconditionally link `UniformTypeIdentifiers` in `OTHER_LDFLAGS`).

    * Tools/TestWebKitAPI/Tests/WebKitCocoa/CopyHTML.mm:

    Add an API test that simulates copying a relative URL (`/downloads`) in a document with a `base` and
    pasting in an app that converts the web archive data to `NSAttributedString`; the resulting link URL
    corresponding to `NSLinkAttributeName` in the attributed string should point to the original URL
    (`https://webkit.org/downloads`), instead of `file:///downloads`.

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

Identifier: 272448.540@safari-7618-branch
  • Loading branch information
whsieh authored and Dan Robson committed Feb 13, 2024
1 parent fb8027f commit 44ac802
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CONSOLE MESSAGE: text/plain: This test verifies that we can get text/html from the clipboard during an onpaste event.
CONSOLE MESSAGE: text/html: <meta charset="UTF-8"><span style="...">This test verifies that we can get text/html from the clipboard during an onpaste event.<span class="Apple-converted-space"> </span></span>
CONSOLE MESSAGE: text/html: <head><meta charset="UTF-8"></head><span style="...">This test verifies that we can get text/html from the clipboard during an onpaste event.<span class="Apple-converted-space"> </span></span>
This test verifies that we can get text/html from the clipboard during an onpaste event. This test requires DRT.
Paste content in this div. This test verifies that we can get text/html from the clipboard during an onpaste event. 
PASS
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CONSOLE MESSAGE: text/plain: This test verifies that we can get text/html from the clipboard during an onpaste event.
CONSOLE MESSAGE: text/html: <meta charset="UTF-8"><span style="...">This test verifies that we can get text/html from the clipboard during an onpaste event.<span class="Apple-converted-space"> </span></span>
CONSOLE MESSAGE: text/html: <head><meta charset="UTF-8"></head><span style="...">This test verifies that we can get text/html from the clipboard during an onpaste event.<span class="Apple-converted-space"> </span></span>
This test verifies that we can get text/html from the clipboard during an onpaste event. This test requires DRT.
Paste content in this div.This test verifies that we can get text/html from the clipboard during an onpaste event. 
PASS
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
CONSOLE MESSAGE: text/plain: This test verifies that we can get text/html from the drag object during an ondrop event.
CONSOLE MESSAGE: text/html: <meta charset="UTF-8"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-size: medium; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; display: inline !important; float: none;">This test verifies that we can get text/html from the drag object during an ondrop event.<span class="Apple-converted-space"> </span></span>
CONSOLE MESSAGE: text/html: <head><meta charset="UTF-8"></head><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-size: medium; font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none; display: inline !important; float: none;">This test verifies that we can get text/html from the drag object during an ondrop event.<span class="Apple-converted-space"> </span></span>
This test verifies that we can get text/html from the drag object during an ondrop event. This test requires DRT.
PASS
39 changes: 27 additions & 12 deletions Source/WebCore/editing/markup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "FrameLoader.h"
#include "HTMLAttachmentElement.h"
#include "HTMLBRElement.h"
#include "HTMLBaseElement.h"
#include "HTMLBodyElement.h"
#include "HTMLDivElement.h"
#include "HTMLHeadElement.h"
Expand Down Expand Up @@ -333,10 +334,27 @@ class StyledMarkupAccumulator final : public MarkupAccumulator {
return node.parentOrShadowHostNode();
}

void prependMetaCharsetUTF8TagIfNonASCIICharactersArePresent()
void prependHeadIfNecessary(const HTMLBaseElement* baseElement)
{
if (!containsOnlyASCII())
#if PLATFORM(COCOA)
// On Cocoa platforms, this markup is eventually persisted to the pasteboard and read back as UTF-8 data,
// so this meta tag is needed for clients that read this data in the future from the pasteboard and load it.
bool shouldAppendMetaCharset = !containsOnlyASCII();
#else
bool shouldAppendMetaCharset = false;
#endif
if (!shouldAppendMetaCharset && !baseElement)
return;

m_reversedPrecedingMarkup.append("</head>"_s);
if (baseElement) {
StringBuilder markupForBase;
appendStartTag(markupForBase, *baseElement, false, DoesNotFullySelectNode);
m_reversedPrecedingMarkup.append(markupForBase.toString());
}
if (shouldAppendMetaCharset)
m_reversedPrecedingMarkup.append("<meta charset=\"UTF-8\">"_s);
m_reversedPrecedingMarkup.append("<head>"_s);
}

private:
Expand Down Expand Up @@ -975,7 +993,7 @@ static RefPtr<Node> highestAncestorToWrapMarkup(const Position& start, const Pos
}

static String serializePreservingVisualAppearanceInternal(const Position& start, const Position& end, Vector<Ref<Node>>* nodes, ResolveURLs resolveURLs, SerializeComposedTree serializeComposedTree, IgnoreUserSelectNone ignoreUserSelectNone,
AnnotateForInterchange annotate, ConvertBlocksToInlines convertBlocksToInlines, StandardFontFamilySerializationMode standardFontFamilySerializationMode, MSOListMode msoListMode)
AnnotateForInterchange annotate, ConvertBlocksToInlines convertBlocksToInlines, StandardFontFamilySerializationMode standardFontFamilySerializationMode, MSOListMode msoListMode, PreserveBaseElement preserveBaseElement)
{
static NeverDestroyed<const String> interchangeNewlineString(MAKE_STATIC_STRING_IMPL("<br class=\"" AppleInterchangeNewline "\">"));

Expand Down Expand Up @@ -1064,11 +1082,8 @@ static String serializePreservingVisualAppearanceInternal(const Position& start,
if (annotate == AnnotateForInterchange::Yes && needInterchangeNewlineAfter(visibleEnd.previous()))
accumulator.append(interchangeNewlineString.get());

#if PLATFORM(COCOA)
// On Cocoa platforms, this markup is eventually persisted to the pasteboard and read back as UTF-8 data,
// so this meta tag is needed for clients that read this data in the future from the pasteboard and load it.
accumulator.prependMetaCharsetUTF8TagIfNonASCIICharactersArePresent();
#endif
RefPtr baseElement = preserveBaseElement == PreserveBaseElement::Yes ? document->firstBaseElement() : nullptr;
accumulator.prependHeadIfNecessary(baseElement.get());

return accumulator.takeResults();
}
Expand All @@ -1077,13 +1092,13 @@ String serializePreservingVisualAppearance(const SimpleRange& range, Vector<Ref<
{
return serializePreservingVisualAppearanceInternal(makeDeprecatedLegacyPosition(range.start), makeDeprecatedLegacyPosition(range.end),
nodes, resolveURLs, SerializeComposedTree::No, IgnoreUserSelectNone::No,
annotate, convertBlocksToInlines, StandardFontFamilySerializationMode::Keep, MSOListMode::DoNotPreserve);
annotate, convertBlocksToInlines, StandardFontFamilySerializationMode::Keep, MSOListMode::DoNotPreserve, PreserveBaseElement::No);
}

String serializePreservingVisualAppearance(const VisibleSelection& selection, ResolveURLs resolveURLs, SerializeComposedTree serializeComposedTree, IgnoreUserSelectNone ignoreUserSelectNone, Vector<Ref<Node>>* nodes)
String serializePreservingVisualAppearance(const VisibleSelection& selection, ResolveURLs resolveURLs, SerializeComposedTree serializeComposedTree, IgnoreUserSelectNone ignoreUserSelectNone, PreserveBaseElement preserveBaseElement, Vector<Ref<Node>>* nodes)
{
return serializePreservingVisualAppearanceInternal(selection.start(), selection.end(), nodes, resolveURLs, serializeComposedTree, ignoreUserSelectNone,
AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, StandardFontFamilySerializationMode::Keep, MSOListMode::DoNotPreserve);
AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, StandardFontFamilySerializationMode::Keep, MSOListMode::DoNotPreserve, preserveBaseElement);
}

static bool shouldPreserveMSOLists(StringView markup)
Expand All @@ -1109,7 +1124,7 @@ String sanitizedMarkupForFragmentInDocument(Ref<DocumentFragment>&& fragment, Do

// SerializeComposedTree::No because there can't be a shadow tree in the pasted fragment.
auto result = serializePreservingVisualAppearanceInternal(firstPositionInNode(bodyElement.get()), lastPositionInNode(bodyElement.get()), nullptr,
ResolveURLs::YesExcludingURLsForPrivacy, SerializeComposedTree::No, IgnoreUserSelectNone::No, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, StandardFontFamilySerializationMode::Strip, msoListMode);
ResolveURLs::YesExcludingURLsForPrivacy, SerializeComposedTree::No, IgnoreUserSelectNone::No, AnnotateForInterchange::Yes, ConvertBlocksToInlines::No, StandardFontFamilySerializationMode::Strip, msoListMode, PreserveBaseElement::No);

if (msoListMode != MSOListMode::Preserve)
return result;
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/editing/markup.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ ExceptionOr<void> replaceChildrenWithFragment(ContainerNode&, Ref<DocumentFragme
enum class ConvertBlocksToInlines : bool { No, Yes };
enum class SerializeComposedTree : bool { No, Yes };
enum class IgnoreUserSelectNone : bool { No, Yes };
enum class PreserveBaseElement : bool { No, Yes };
WEBCORE_EXPORT String serializePreservingVisualAppearance(const SimpleRange&, Vector<Ref<Node>>* = nullptr, AnnotateForInterchange = AnnotateForInterchange::No, ConvertBlocksToInlines = ConvertBlocksToInlines::No, ResolveURLs = ResolveURLs::No);
String serializePreservingVisualAppearance(const VisibleSelection&, ResolveURLs = ResolveURLs::No, SerializeComposedTree = SerializeComposedTree::No,
IgnoreUserSelectNone = IgnoreUserSelectNone::Yes, Vector<Ref<Node>>* = nullptr);
IgnoreUserSelectNone = IgnoreUserSelectNone::Yes, PreserveBaseElement = PreserveBaseElement::No, Vector<Ref<Node>>* = nullptr);

enum class SerializedNodes : uint8_t { SubtreeIncludingNode, SubtreesOfChildren };
enum class SerializationSyntax : uint8_t { HTML, XML };
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/loader/archive/cf/LegacyWebArchive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ RefPtr<LegacyWebArchive> LegacyWebArchive::createFromSelection(LocalFrame* frame
builder.append(documentTypeString(*document));

Vector<Ref<Node>> nodeList;
builder.append(serializePreservingVisualAppearance(frame->selection().selection(), ResolveURLs::No, SerializeComposedTree::Yes, IgnoreUserSelectNone::Yes, &nodeList));
builder.append(serializePreservingVisualAppearance(frame->selection().selection(), ResolveURLs::No, SerializeComposedTree::Yes, IgnoreUserSelectNone::Yes, PreserveBaseElement::Yes, &nodeList));

auto archive = create(builder.toString(), *frame, WTFMove(nodeList), nullptr);
if (!archive)
Expand Down
12 changes: 1 addition & 11 deletions Tools/TestWebKitAPI/Configurations/TestWebKitAPI.xcconfig
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,6 @@ WK_SYSTEM_LDFLAGS_IOS_SINCE_15 = -framework System;
WK_UIKITMACHELPER_LDFLAGS = $(WK_UIKITMACHELPER_LDFLAGS_$(WK_PLATFORM_NAME));
WK_UIKITMACHELPER_LDFLAGS_maccatalyst = -framework UIKitMacHelper;

WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS = $(WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_$(WK_PLATFORM_NAME));
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_iphoneos = $(WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS$(WK_IOS_14));
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_iphonesimulator = $(WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS$(WK_IOS_14));
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_maccatalyst = $(WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS$(WK_IOS_14));
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_IOS_SINCE_14 = -framework UniformTypeIdentifiers;
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_watchos = -framework UniformTypeIdentifiers;
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_watchsimulator = -framework UniformTypeIdentifiers;
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_appletvos = -framework UniformTypeIdentifiers;
WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS_appletvsimulator = -framework UniformTypeIdentifiers;

WK_WEBCORE_LDFLAGS = $(WK_WEBCORE_LDFLAGS_$(WK_PLATFORM_NAME));
WK_WEBCORE_LDFLAGS_iphoneos = -framework WebCore
WK_WEBCORE_LDFLAGS_iphonesimulator = -framework WebCore
Expand All @@ -105,7 +95,7 @@ WK_IMAGEIO_LDFLAGS_MACOS_SINCE_1300 = -framework ImageIO;

OTHER_CPLUSPLUSFLAGS = $(inherited) -isystem $(SDKROOT)/System/Library/Frameworks/System.framework/PrivateHeaders;

OTHER_LDFLAGS = $(inherited) -lgtest -force_load $(BUILT_PRODUCTS_DIR)/libTestWebKitAPI.a -framework JavaScriptCore -framework WebCore -framework WebKit -lWebCoreTestSupport -framework Metal -framework IOSurface $(WK_APPSERVERSUPPORT_LDFLAGS) $(WK_AUTHKIT_LDFLAGS) -framework Network -framework UniformTypeIdentifiers -framework CoreFoundation -framework CoreServices $(WK_HID_LDFLAGS) $(WK_IMAGEIO_LDFLAGS) $(WK_OPENGL_LDFLAGS) $(WK_PDFKIT_LDFLAGS) $(WK_SYSTEM_LDFLAGS) $(WK_UIKITMACHELPER_LDFLAGS) $(WK_UNIFORM_TYPE_IDENTIFIERS_LDFLAGS) $(WK_VISIONKITCORE_LDFLAGS) $(WK_WEBCORE_LDFLAGS) $(WK_REVEAL_LDFLAGS) $(OTHER_LDFLAGS_PLATFORM_$(WK_COCOA_TOUCH));
OTHER_LDFLAGS = $(inherited) -lgtest -force_load $(BUILT_PRODUCTS_DIR)/libTestWebKitAPI.a -framework JavaScriptCore -framework WebCore -framework WebKit -lWebCoreTestSupport -framework Metal -framework IOSurface $(WK_APPSERVERSUPPORT_LDFLAGS) $(WK_AUTHKIT_LDFLAGS) -framework Network -framework UniformTypeIdentifiers -framework CoreFoundation -framework CoreServices $(WK_HID_LDFLAGS) $(WK_IMAGEIO_LDFLAGS) $(WK_OPENGL_LDFLAGS) $(WK_PDFKIT_LDFLAGS) $(WK_SYSTEM_LDFLAGS) $(WK_UIKITMACHELPER_LDFLAGS) $(WK_VISIONKITCORE_LDFLAGS) $(WK_WEBCORE_LDFLAGS) $(WK_REVEAL_LDFLAGS) $(OTHER_LDFLAGS_PLATFORM_$(WK_COCOA_TOUCH));
OTHER_LDFLAGS_PLATFORM_ = -framework Cocoa -framework Carbon;

// FIXME: This should not be built on iOS. Instead we should create and use a TestWebKitAPI application.
Expand Down
45 changes: 45 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/CopyHTML.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,13 @@

#import "PasteboardUtilities.h"
#import "PlatformUtilities.h"
#import "Test.h"
#import "TestWKWebView.h"
#import <UniformTypeIdentifiers/UniformTypeIdentifiers.h>
#import <WebCore/ColorCocoa.h>
#import <WebKit/NSAttributedStringPrivate.h>
#import <WebKit/WKPreferencesPrivate.h>
#import <pal/spi/cocoa/NSAttributedStringSPI.h>
#import <wtf/RetainPtr.h>
#import <wtf/text/WTFString.h>

Expand Down Expand Up @@ -168,6 +172,47 @@ @interface WKWebView () <NSServicesMenuRequestor>
}
}

TEST(CopyHTML, SanitizationPreservesRelativeURLInAttributedString)
{
auto webView = createWebViewWithCustomPasteboardDataEnabled();
[webView synchronouslyLoadHTMLString:@"<!DOCTYPE html>"
"<html>"
"<head><base href='https://webkit.org/' /></head>"
"<body><a href='/downloads'>Click</a> for downloads</body>"
"</html>"];
[webView stringByEvaluatingJavaScript:@"getSelection().selectAllChildren(document.body)"];
[webView copy:nil];
[webView waitForNextPresentationUpdate];

#if PLATFORM(IOS_FAMILY)
RetainPtr archiveData = [UIPasteboard.generalPasteboard dataForPasteboardType:UTTypeWebArchive.identifier];
#else
RetainPtr archiveData = [NSPasteboard.generalPasteboard dataForType:UTTypeWebArchive.identifier];
#endif

__block bool done = false;
__block RetainPtr<NSAttributedString> resultString;
__block RetainPtr<NSError> resultError;
[NSAttributedString _loadFromHTMLWithOptions:@{ } contentLoader:^(WKWebView *loadingWebView) {
return [loadingWebView loadData:archiveData.get() MIMEType:UTTypeWebArchive.preferredMIMEType characterEncodingName:@"" baseURL:[NSURL URLWithString:@"about:blank"]];
} completionHandler:^(NSAttributedString *string, NSDictionary *, NSError *error) {
resultString = string;
resultError = error;
done = true;
}];
TestWebKitAPI::Util::run(&done);

auto links = adoptNS([NSMutableArray<NSURL *> new]);
[resultString enumerateAttribute:NSLinkAttributeName inRange:NSMakeRange(0, 5) options:0 usingBlock:^(NSURL *url, NSRange, BOOL*) {
[links addObject:url];
}];

EXPECT_NULL(resultError);
EXPECT_WK_STREQ("Click for downloads", [resultString string]);
EXPECT_EQ(1U, [links count]);
EXPECT_WK_STREQ("https://webkit.org/downloads", [links firstObject].absoluteString);
}

#if PLATFORM(MAC)

TEST(CopyHTML, ItemTypesWhenCopyingWebContent)
Expand Down

0 comments on commit 44ac802

Please sign in to comment.