1368077 - Dragging URLs should strip page titles from the URL on the pasteboard

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Stephen A Pohl [:spohl]]

Bug 1358075 restored the ability to drag URLs to Finder and have the resulting .webloc files named based on the URL's title. However, it no longer stripped the page title from the URL on the pasteboard, resulting in incorrect URLs being stored in the .webloc file. For example, dragging and dropping https://www.google.com/ on Finder would result in a .webloc file that would open https://www.google.com/%0AGoogle. Depending on the page title and/or URL, this could also cause the drop of the URL to fail completely, as was reported in bug 1358075 comment 21.

Comment 1

[Stephen A Pohl [:spohl]]

Attachment: Patch

Patch 1 in bug 1358075 (attachment 8867833 [details] [diff] [review]) moved the escaping of the URL and assignment of nativeURL ahead of the truncating of the URL in case a new line was found. This was done because we needed nativeURL inside the if (newlinePos >= 0) block to set urlsAndTitles on the pasteboard, but it accidentally resulted in urls+titles being stored as URL. This patch reorganizes this code to restore proper functionality and does away with an alloc/release.

Comment 2

[Stephen A Pohl [:spohl]]

Attachment: Patch

Forgot to qrefresh the previous patch. This patch only sets public.url-name and WebURLsWithTitlesPboardType if we actually have a title, which was the previous behavior.

Comment 3

[Markus Stange [:mstange]]

Comment on attachment 8871781 [details] [diff] [review]
Patch

Review of attachment 8871781 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/cocoa/nsClipboard.mm
@@ +690,5 @@
>        urlObject->GetData(url);
>  
> +      NSString* nativeTitle = nil;
> +
> +      // A newline embedded in the URL means that the form is actually URL + title.

Maybe add a comment that those combined strings are assembled in nsDragService::GetData.

Comment 4

[Stephen A Pohl [:spohl]]

Attachment: Patch

Thanks, Markus! Added comment, carrying over r+.

Comment 5

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b9b12d91081fbbe98b82b2501834c6877366011
Bug 1368077: Strip page titles from URLs on the pasteboard when dragging on OSX/macOS. r=mstange

Comment 6

[Stephen A Pohl [:spohl]]

Comment on attachment 8871786 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1358075
[User impact if declined]: When URLs are dragged to Finder, the resulting .webloc file will have an incorrect URL or the drop will fail.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: Yes. Same steps as bug 1358075, but we should also verify:
1. that the issues reported in bug 1358075 comment 21 no longer occur, and
2. that when the .webloc file is opened, the correct URL is opened in the browser (no page title at the end of the URL).
[List of other uplifts needed for the feature/fix]: bug 1358075, bug 1330470, bug 1361116
[Is the change risky?]: no
[Why is the change risky/not risky?]: This restores the behavior that existed prior to the patches in bug 1358075 landing.
[String changes made/needed]: none

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4b9b12d91081

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8871786 [details] [diff] [review]
Patch

regression from bug 1358075, beta54+

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3f0075d62b36

Comment 10

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce the issue described in comment 6 using Firefox 53.0.3 (Build Id:20170518000419).

This issue is verified fixed on Firefox 54.0 (Build Id:20170605134926) and Firefox 55.0a1 (Build Id:20170607030206) using macOS 10.12.1 and macOS 10.11.6.

Comment 11

[Aki Sasaki (not active)]

https://hg.mozilla.org/projects/jamun/rev/3f0075d62b3675ae8c8e25552336a8091f08cf3e
Bug 1368077 - Strip page titles from URLs on the pasteboard when dragging on OSX/macOS. r=mstange, a=jcristau