1366327 - Firefox copy URL and email link features should deal with Reader View URLs properly.

Status: VERIFIED FIXED

(Firefox :: Address Bar, enhancement, P1)

Description

[Caspy7]

With 57 getting the page action menu, it seems likely that a lot more people are going to be exposed to and use share actions such as Email, Copy, etc. And from a mockup I saw, Reader Mode may be getting even more visibility.

Currently if you attempt to share any page while in Reader View instead of sending the URL you see, you get something like about:reader?url=https%3A%2F%2Fdolske.wordpress.com%2F2017%2F05%2F18%2Fphoton-engineering-newsletter-1%2F
This makes for a poor experience for users - and potentially frustrating/confusing experience for recipients. 

When I previously brought up that addons were fetching this internal URL, it was pointed out that this was intentional and addons would have to convert it on their own for user-facing purposes. I doubt any did, but notably *we* didn't do it internally for something like Email (Copy was fixed though).

I've just now seen that the "Share" button is disabled in Reader View (don't think it was originally). I'd argue that due to it's increased visibility, this should be enabled for Reader View. As seen in the following mockup, the Share button now seems to pass off URLs straight to the platform's sharing function.
http://i.imgur.com/EYGFt3h.jpg
We can't expect any of these to convert the URL, so let's just convert it for them.

Comment 1

[:Gijs (he/him)]

Bob, any idea if the "add-ons should convert it themselves" thing is still what we want to do for webextensions? Could we do something better? Do we know what Edge and Safari do here?

Comment 2

[Andy McKay]

I'm not sure what the "convert it themselves" thing is either. Could you elaborate on that Caspy?

There's a few bugs about providing reader mode here, it's something I'd like to see improved support for in Firefox bug 1286387.

Comment 3

[:Gijs (he/him)]

(In reply to Andy McKay [:andym] from comment #2)
> I'm not sure what the "convert it themselves" thing is either. Could you
> elaborate on that Caspy?

Basically, add-ons have access to the URI loaded in tabs via various APIs. The question is, if the tab is in reader mode, should they get the original URI of the page, or the (internal, not normally user-exposed) about:reader URI that looks like this:

about:reader?url=url_encoded_version_of_original_uri

The latter gives more information, but obviously the former may be more "what add-ons expect". If the URI gets reused (via copy/paste or sharing to twitter or whatever) then providing the original URI makes more sense. On the other hand, for security decisions etc., providing the about: URI is correct. It's not clear what other browsers do for this in terms of what they expose to add-ons (Edge, for instance, internally uses "read:http://..." for reader-ized pages. Not sure what Safari does. Not sure how either of this do/don't expose this to add-ons).

Comment 4

[Caspy7]

In retrospect maybe I should have avoided the addons topic as I don't know that that should be the drive here.

Originally reader mode had not been as thoroughly locked down as it is now. IIRC you could share a link via twitter, but it would be the about:reader?url=https%3A%2F... type. Now you cannot share at all or simply email a link from reader mode. 

We should turn on sharing again and convert the path to a normal URL as it's frustrating when you'd like to share a link (or even just send it to another Firefox device) and you're prevented from doing so.

Comment 5

[Bob Silverberg [:bsilverberg]]

Thanks for replying, Andy, as I didn't really understand how this relates to WebExtensions. Removing my needinfo from this now.

Comment 6

[Andy McKay]

From a WebExtension point of view, I think it would be nice if you were able to tell if a tab was in reader mode or not, then the extension author could choose what URL to share. I've added in bug 1381992 as a first step towards that.

Comment 7

[:Gijs (he/him)]

The share plane was removed, bug 1388902. But email link and copy URL both copy about:reader? URLs now, and shouldn't. I can fix that.

Comment 8

[:Gijs (he/him)]

Attachment: Bug 1366327 - deal with about:reader, IDNA and non-exposable URIs better for copy/email link,

Review commit: https://reviewboard.mozilla.org/r/169656/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/169656/

Comment 9

[Drew Willcoxon :adw]

Comment on attachment 8898285 [details]
Bug 1366327 - deal with about:reader, IDNA and non-exposable URIs better for copy/email link,

https://reviewboard.mozilla.org/r/169656/#review175010

lgtm

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9b13c236beb
deal with about:reader, IDNA and non-exposable URIs better for copy/email link, r=adw

Comment 11

[Caspy7]

Original design docs presented a share item in the Action menu to pass off pages to the platform's share function. Can anyone speak to if that's still planned? And if so does that only involve sending the URL?
If so, we'd probably want to do the same with it.

Comment 12

[:Gijs (he/him)]

(In reply to Caspy7 from comment #11)
> Original design docs presented a share item in the Action menu to pass off
> pages to the platform's share function. Can anyone speak to if that's still
> planned?

This is covered in bug 1363180, bug 1363168, bug 1363169 . I don't think it will make 57. It needs platform works and all our engineers who can do that work are very busy at the moment.

> And if so does that only involve sending the URL?
> If so, we'd probably want to do the same with it.

We will be sure to keep it in mind, but obviously I can't write code for something that doesn't exist.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c9b13c236beb

Comment 14

[u554753]

Hey Gijs, can you clarify the STR needed to be able to verify this bug?

Comment 15

[:Gijs (he/him)]

(In reply to Grover Wimberly IV [:Grover-QA] from comment #14)
> Hey Gijs, can you clarify the STR needed to be able to verify this bug?

Sure!

STR:

1. go to the bbc news website ( http://www.bbc.co.uk/news )
2. open an article
3. enter reader mode
4. in the page action menu, click 'copy url'
5. paste the result and check it.

Expected:
should be the URL of the article

Actual (prior to this patch):

about:reader?url=....

Comment 16

[:Gijs (he/him)]

(similar steps with 'email link' to check if the contents of the email have the correct article link, not "about:reader?url=...")

Comment 17

[u554753]

Verified on 58 with Windows, Mac, and Linux.

Comment 18

[Emil Ghitta, QA [:emilghitta]]

I have reproduced the issue mentioned in comment 0, using an affected Firefox 55.0a1 build (BuildId:20170519030205).

I have verified that the issue is not reproducible using Firefox 57.0b6 (Build Id:20171005195903) on Windows 10 64bit, macOS 10.11.6 and Ubuntu 16.04 64bit.