1359861 - Unable to send page to device or find page in "synced tabs" when in reading mode

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P2)

Description

[Mark Hammond [:markh] [:mhammond]]

STR:
* open a page in reading mode.
* right-click

expected:
* "Send Page to Device" is available and works.

actual:
* It isn't available.

We should probably send the original URL rather than the about:reader URL - bug 1359650 has the same considerations re whether to use the about URL or the actual URL.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

(In reply to Mark Hammond [:markh] from comment #0)
> We should probably send the original URL rather than the about:reader URL -

I was chatting with Grisha about this and he suggests the about:reader URL probably *is* what we should send, but this assumes Fennec and iOS can do the right thing with these URLs. Fennec seems to, but we are unsure about iOS. Steph, WDYT?

Comment 2

[Stephan Leroux [:sleroux]]

> the about:reader URL probably *is* what we should send, but this assumes Fennec and iOS can do the right thing with these URLs

I don't believe iOS handles about:reader URLs the same way Desktop/Fennec does. The way we go about handling reader mode is through Readability which generates our reader mode page that is served from a local web server. I think Desktop/Fennec might use Gecko for this?

Adding :st3fan as he is more familiar with the reader mode implementation.

Comment 3

[:Grisha Kruglov]

Is it possible to special-case incoming about:reader URLs so that they can be treated as Readability pages on iOS?

Perhaps perform a URL transformation here: https://github.com/mozilla-mobile/firefox-ios/blob/d22e8dc969cada55f7ca2f2489e90740035186ac/Client/Frontend/Reader/ReaderMode.js#L216-L221 ? - but I'm unsure what implications that might have.

Being able to send pages opened in reability mode from device to device seems like a great feature; I think it carries user intent very well.

Comment 4

[Stephan Leroux [:sleroux]]

Clearing NI. Probably needs a re-triage.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Bug 1359861 was specifically "synced tabs", but that's now a dupe of this, so this is now about both "send to device" and "synced tabs"

Comment 8

[Mark Hammond [:markh] [:mhammond]]

(In reply to Mark Hammond [:markh] from comment #7)
> Bug 1359861 was specifically "synced tabs", but that's now a dupe of this,
> so this is now about both "send to device" and "synced tabs"

Oops - make that bug 1359650

Comment 10

[Lina Butler (ex-Mozilla)]

Grisha, what are the next steps here?

Comment 11

[:Grisha Kruglov]

Land iOS patch in https://github.com/mozilla-mobile/firefox-ios/pull/2819 (iirc it looked good but might have required addressing some nits), and then allow sending about:reader URLs on Desktop.

Comment 13

[Farhan Patel [:farhan]]

Just landed the iOS patch. https://github.com/mozilla-mobile/firefox-ios/pull/3962

Comment 14

[Carol Ng [:cng]]

Attachment: Github pull request

Comment 15

[Mark Hammond [:markh] [:mhammond]]

So this looks like it was actionable about a year ago, but now we have Fenix to consider :) It looks like fenix has a reader mode, but they don't have synced tabs yet - but they are very close to landing "send to device". We could probably try and get this fixed now, which will basically "force" fenix to consider about:reader URLs now for "send tab" and later for "synced tabs".

cc vlad who is working on "send tab" for Fenix.

Comment 16

[Ryan Feeley [:rfeeley]]

We should probably send the original URL rather than the about:reader URL

Mark: this is the prefered user experience, and based on my experience with Fenix, incoming URLs often require browser choice before opening. I'm not sure what they would do with about:reader URLs but let's send the original URL from every platform.

Comment 17

[Ryan Kelly [:rfkelly]]

incoming URLs often require browser choice before opening

Yep, Fenix will open incoming sent tabs in your default browser, which may not be Fenix.

Comment 18

[:Gijs (he/him)]

Can we detect whether the device we're sending to is desktop and send the about:reader URL there to persist reader-ness? I think that'd be user expectation... I assume we want to reuse this bug for desktop?

Comment 19

[Mark Hammond [:markh] [:mhammond]]

We could detect desktop->desktop, but comment 16 seems fairly clear, and I see no reasonable argument for desktop->desktop that doesn't apply for desktop->anything or for anything->desktop. But to be clear - Ryan, are you saying that the original URL should be used (so it's not opened in reader mode) even for desktop->desktop sends?

Comment 20

[Ryan Feeley [:rfeeley]]

But to be clear - Ryan, are you saying that the original URL should be used (so it's not opened in reader mode) even for desktop->desktop sends?

Correct. Seems like the most expected, and least fragile thing to do.

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:markh, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 22

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is relatively low, S3. However, the bug has 4 duplicates.
:skhamis, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 23

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 24

[Mark Hammond [:markh] [:mhammond]]

The implementation should be able to use ReaderMode.getOriginalUrl() to extract the original URL.

For synced-tabs, the page is excluded as a side-effect of excluding all about: pages by the check here - so we'd just carve out an exception for about:reader? and call getOriginalUrl()

For send-tab, I think carving out a similar exception here is what we need to do. However, something then needs to call getOriginalUrl() - it might make sense to change the signature of that to be, say getsShareableURL(url) which returns a URL or null.

And tests :)

Comment 25

[Mark Hammond [:markh] [:mhammond]]

Attachment: Bug 1359861 - allow reader-mode tabs to be shared. r?Gijs,skhamis

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d82c03e04249
allow reader-mode tabs to be shared. r=Gijs,skhamis

Comment 27

[Norisz Fay [:noriszfay]]

Backed out for causing xpcshell failure on test_BrowserUtils.js

Backout link

Push with failures

Failure log

Comment 28

[Mark Hammond [:markh] [:mhammond]]

Oops, android keeps burning me - https://treeherder.mozilla.org/jobs?repo=try&revision=13c022052769879c47b263f62c8cca5d24a45d8d is a try with the failing new test skipped on Android.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c009325029cd
allow reader-mode tabs to be shared. r=Gijs,skhamis

Comment 30

[Iulian Moraru]

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