Skip to content

Commit

Permalink
Revert "[Clipboard] Don't add meta charset tag for async write() meth…
Browse files Browse the repository at this point in the history
…od on Mac."

This reverts commit 0a05892.

Reason for revert: Caused regressions during copy. crbug.com/333989067.

Original change's description:
> [Clipboard] Don't add meta charset tag for async write() method on Mac.
>
> Remove the logic from the browser process that prepends a meta
> charset tag to the HTML markup before writing to the pasteboard.
> Instead, add the meta charset tag only for copy/paste commands in the
> Blink code.
>
> Bug: 1504501
>
> Change-Id: I95ec6e2dc6395076af78bfbbba78b8579ff16857
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5187335
> Reviewed-by: Avi Drissman <[email protected]>
> Reviewed-by: Koji Ishii <[email protected]>
> Commit-Queue: Anupam Snigdha <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1257511}

(cherry picked from commit 7bf0ba4)

Bug: 1504501, 333989067
Change-Id: I703e2dcd3c5bd8d2ef6a3da44ce17a7e25218d96
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5455853
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Anupam Snigdha <[email protected]>
Reviewed-by: Koji Ishii <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#1288202}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5463860
Commit-Queue: Koji Ishii <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
Auto-Submit: Anupam Snigdha <[email protected]>
Cr-Commit-Position: refs/branch-heads/6422@{#58}
Cr-Branched-From: 9012208-refs/heads/main@{#1287751}
  • Loading branch information
snianu authored and Chromium LUCI CQ committed Apr 18, 2024
1 parent b3d3684 commit ebdb716
Show file tree
Hide file tree
Showing 9 changed files with 18 additions and 141 deletions.
26 changes: 0 additions & 26 deletions content/browser/renderer_host/clipboard_host_impl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,30 +252,4 @@ IN_PROC_BROWSER_TEST_F(ClipboardBrowserTest, NumberOfFormatsOnRead) {
1);
}

IN_PROC_BROWSER_TEST_F(ClipboardBrowserTest, CopyPasteHtmlOnMac) {
NavigateAndSetFocusToPage();
base::RunLoop loop;
// Execute copy command to select the content of the body element.
ASSERT_TRUE(ExecJs(shell(),
" document.execCommand('selectAll');"
" document.execCommand('copy')"));
loop.RunUntilIdle();
// Get the HTML content from the clipboard and check that meta tag is added.
std::u16string html;
std::string url;
uint32_t ignore_offsets;
ui::Clipboard::GetForCurrentThread()->ReadHTML(
ui::ClipboardBuffer::kCopyPaste, nullptr, &html, &url, &ignore_offsets,
&ignore_offsets);
// The meta tag is added only on Mac. See AddMetaCharsetTagToHtmlOnMac for
// details.
#if BUILDFLAG(IS_MAC)
EXPECT_TRUE(base::UTF16ToUTF8(html).find("<meta charset=\"utf-8\">") !=
std::string::npos);
#else
EXPECT_TRUE(base::UTF16ToUTF8(html).find("<meta charset=\"utf-8\">") ==
std::string::npos);
#endif
}

} // namespace content
46 changes: 0 additions & 46 deletions third_party/blink/renderer/core/clipboard/clipboard_utilities.cc
Original file line number Diff line number Diff line change
Expand Up @@ -104,50 +104,4 @@ String PNGToImageMarkup(const mojo_base::BigBuffer& png_data) {
return markup.ToString();
}

// NSPasteboardTypeHTML does not define what encoding should be used, and if
// no character encoding is specified, it is likely that the data will be
// interpreted as ISO-8859-1, even with modern releases like macOS 14.2.
// (https://crbug.com/11957)
//
// (Note that Safari does not encounter this issue because in addition to
// adding the data to the pasteboard as a NSPasteboardTypeHTML flavor, it also
// adds it as a UTTypeWebArchive type which includes the encoding.)
//
// This issue has been filed as FB13522476. When this feedback is addressed
// and NSPasteboardTypeHTML is interpreted as UTF-8, remove the code that adds
// a charset declaration.
#if BUILDFLAG(IS_MAC)
static constexpr char kMetaTag[] = "<meta charset=\"utf-8\">";
static constexpr unsigned kMetaTagLength = sizeof(kMetaTag) - 1;
#endif

String AddMetaCharsetTagToHtmlOnMac(const String& html) {
#if BUILDFLAG(IS_MAC)
if (html.Find(kMetaTag) == kNotFound) {
StringBuilder html_result;
html_result.Append(kMetaTag);
html_result.Append(html);
return html_result.ToString();
}
#endif
return html;
}

String RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac(
const String& html,
unsigned& fragment_start,
unsigned& fragment_end) {
#if BUILDFLAG(IS_MAC)
DCHECK_EQ(fragment_start, 0u);
DCHECK_EQ(fragment_end, html.length());
if (html.StartsWith(kMetaTag)) {
const String html_fragment = html.Substring(kMetaTagLength);
fragment_start = 0;
fragment_end = html_fragment.length();
return html_fragment;
}
#endif
return html;
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,6 @@ CORE_EXPORT void ReplaceNBSPWithSpace(String&);
CORE_EXPORT String ConvertURIListToURL(const String& uri_list);
CORE_EXPORT String URLToImageMarkup(const KURL&, const String& title);
CORE_EXPORT String PNGToImageMarkup(const mojo_base::BigBuffer& png_data);
CORE_EXPORT String AddMetaCharsetTagToHtmlOnMac(const String& html);
CORE_EXPORT String
RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac(const String& html,
unsigned& fragment_start,
unsigned& fragment_end);

} // namespace blink

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,39 +72,4 @@ TEST(ClipboardUtilitiesTest, PNGToImageMarkup) {
PNGToImageMarkup(png));
}

TEST(ClipboardUtilitiesTest, AddMetaCharsetTagToHtmlOnMac) {
const String html_markup = "<p>Test</p>";
const String expected_html_markup = "<meta charset=\"utf-8\"><p>Test</p>";
#if BUILDFLAG(IS_MAC)
EXPECT_EQ(AddMetaCharsetTagToHtmlOnMac(html_markup), expected_html_markup);
#else
EXPECT_EQ(AddMetaCharsetTagToHtmlOnMac(html_markup), html_markup);
#endif
}

TEST(ClipboardUtilitiesTest, RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac) {
#if BUILDFLAG(IS_MAC)
const String expected_html_markup = "<p>Test</p>";
const String html_markup = "<meta charset=\"utf-8\"><p>Test</p>";
unsigned fragment_start = 0;
unsigned fragment_end = html_markup.length();
const String actual_value = RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac(
html_markup, fragment_start, fragment_end);
EXPECT_EQ(actual_value, expected_html_markup);
EXPECT_EQ(fragment_start, 0u);
// Meta tag is not part of the copied fragment.
EXPECT_EQ(fragment_end, expected_html_markup.length());
#else
const String html_markup = "<p>Test</p>";
unsigned fragment_start = 0;
unsigned fragment_end = html_markup.length();
const String actual_value = RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac(
html_markup, fragment_start, fragment_end);
// On non-Mac platforms, the HTML markup is unchanged.
EXPECT_EQ(actual_value, html_markup);
EXPECT_EQ(fragment_start, 0u);
EXPECT_EQ(fragment_end, html_markup.length());
#endif
}

} // namespace blink
12 changes: 2 additions & 10 deletions third_party/blink/renderer/core/clipboard/data_object_item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include "third_party/blink/public/mojom/file_system_access/file_system_access_data_transfer_token.mojom-blink.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/core/clipboard/clipboard_mime_types.h"
#include "third_party/blink/renderer/core/clipboard/clipboard_utilities.h"
#include "third_party/blink/renderer/core/clipboard/system_clipboard.h"
#include "third_party/blink/renderer/core/fileapi/blob.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
Expand Down Expand Up @@ -206,15 +205,8 @@ String DataObjectItem::GetAsString() const {
data = system_clipboard_->ReadRTF();
} else if (type_ == kMimeTypeTextHTML) {
KURL ignored_source_url;
unsigned ignored_start = 0;
unsigned ignored_end = 0;
data = system_clipboard_->ReadHTML(ignored_source_url, ignored_start,
ignored_end);
// On Mac, remove meta charset tag that was added for compatibility with
// native apps. See comments in AddMetaCharsetTagToHtmlOnMac for more
// details.
data = RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac(data, ignored_start,
ignored_end);
unsigned ignored;
data = system_clipboard_->ReadHTML(ignored_source_url, ignored, ignored);
} else {
data = system_clipboard_->ReadCustomData(type_);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,7 @@ void ClipboardCommands::WriteSelectionToClipboard(LocalFrame& frame) {
const KURL& url = frame.GetDocument()->Url();
const String html = frame.Selection().SelectedHTMLForClipboard();
String plain_text = frame.SelectedTextForClipboard();
// On Mac, add a meta charset tag for compatibility with native apps.
// See comments in AddMetaCharsetTagToHtmlOnMac for more details.
frame.GetSystemClipboard()->WriteHTML(AddMetaCharsetTagToHtmlOnMac(html), url,
frame.GetSystemClipboard()->WriteHTML(html, url,
GetSmartReplaceOption(frame));
ReplaceNBSPWithSpace(plain_text);
frame.GetSystemClipboard()->WritePlainText(plain_text,
Expand Down Expand Up @@ -462,15 +460,10 @@ ClipboardCommands::GetFragmentFromClipboard(LocalFrame& frame) {
unsigned fragment_start = 0;
unsigned fragment_end = 0;
KURL url;
// On Mac, remove meta charset tag that was added for compatibility with
// native apps. See comments in AddMetaCharsetTagToHtmlOnMac for more
// details.
const String markup =
frame.GetSystemClipboard()->ReadHTML(url, fragment_start, fragment_end);
const String html_markup = RemoveMetaTagAndCalcFragmentOffsetsFromHtmlOnMac(
markup, fragment_start, fragment_end);
fragment = CreateStrictlyProcessedFragmentFromMarkupWithContext(
*frame.GetDocument(), html_markup, fragment_start, fragment_end, url);
*frame.GetDocument(), markup, fragment_start, fragment_end, url);
}
if (fragment)
return std::make_pair(fragment, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,10 +459,7 @@ void WebPluginContainerImpl::Copy() {
return;

LocalFrame* frame = element_->GetDocument().GetFrame();
String html = web_plugin_->SelectionAsMarkup();
// On Mac, add a meta charset tag for compatibility with native apps.
// See comments in AddMetaCharsetTagToHtmlOnMac for more details.
frame->GetSystemClipboard()->WriteHTML(AddMetaCharsetTagToHtmlOnMac(html),
frame->GetSystemClipboard()->WriteHTML(web_plugin_->SelectionAsMarkup(),
KURL());
String text = web_plugin_->SelectionAsText();
ReplaceNBSPWithSpace(text);
Expand Down
11 changes: 7 additions & 4 deletions ui/base/clipboard/clipboard_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -381,10 +381,13 @@
void ClipboardIOS::WriteHTML(
base::StringPiece markup,
std::optional<base::StringPiece> /* source_url */) {
NSDictionary<NSString*, id>* html_item = @{
ClipboardFormatType::HtmlType().ToNSString() :
base::SysUTF8ToNSString(markup)
};
// We need to mark it as utf-8. (see crbug.com/40759159)
std::string html_fragment_str("<meta charset='utf-8'>");
html_fragment_str += markup;
NSString* html = base::SysUTF8ToNSString(html_fragment_str);

NSDictionary<NSString*, id>* html_item =
@{ClipboardFormatType::HtmlType().ToNSString() : html};
[GetPasteboard() addItems:@[ html_item ]];
}

Expand Down
8 changes: 6 additions & 2 deletions ui/base/clipboard/clipboard_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -482,8 +482,12 @@
void ClipboardMac::WriteHTML(
base::StringPiece markup,
std::optional<base::StringPiece> /* source_url */) {
[GetPasteboard() setString:base::SysUTF8ToNSString(markup)
forType:NSPasteboardTypeHTML];
// We need to mark it as utf-8. (see crbug.com/40759159)
std::string html_fragment_str("<meta charset='utf-8'>");
html_fragment_str.append(markup);
NSString* html_fragment = base::SysUTF8ToNSString(html_fragment_str);

[GetPasteboard() setString:html_fragment forType:NSPasteboardTypeHTML];
}

void ClipboardMac::WriteSvg(base::StringPiece markup) {
Expand Down

0 comments on commit ebdb716

Please sign in to comment.