1550144 - Provide API to add custom history entry (e.g. about:reader?url=)

Status: NEW

(GeckoView :: General, enhancement, P3)

Description

[Christian Sadilek [:csadilek]]

Android components provides a new reader view feature implemented as a Web extension. The current reader view functionality in Fennec sets document.location to about:reader?url={originalUrl} so that a later navigation to this page still has reader view enabled.

It's not possible to set document.location to about:reader* in a web extension content script (Security Error).

We'd either need new (Java) API to add a custom history entry or to attach some other history state that will allow us to determine that reader view should be turned on when navigating back/forward.

After discussing this with the GeckoView team we're not sure what the right approach is to solving this.

We could use pushState and not support about:reader going forward, but this doesn't seem ideal.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

:kmag what do you think here? Is there a strategy we haven't considered? It seems like Web Extensions may have an opportunity to offer a more frictionless path for "transformation"-style extensions like reader mode.

Comment 2

[Kris Maglione [:kmag]]

Extensions can replace the current history entry using location.replace or browser.tabs.update. We'd need to change the security rules for the extension to allow it load about:reader, but that's doable.

Comment 3

[Christian Sadilek [:csadilek]]

Thanks, :kmag.

Seems location.replace is documented to not affect the session's history. It would be ideal if that worked. I guess location.assign will work too with the security rules changed?

<We'd need to change the security rules for the extension to allow it load about:reader, but that's doable.>
Sorry, new to this, can you elaborate what's required to make this happen? :)

Comment 4

[Kris Maglione [:kmag]]

Well, the easiest thing to do would be to just make about:reader linkable by anyone:

https://searchfox.org/mozilla-central/rev/197210b8c139b64f642edaa3336d26b9c1761568/netwerk/protocol/about/nsIAboutModule.idl#72

But since we probably don't want to do that, the second easiest thing would be to add a new MAKE_LINKABLE_BY_EXTENSIONS flag that sets the URI_LOADABLE_BY_EXTENSIONS flag here.

The third easiest thing to do would be to only make it linkable by extensions with a certain permission.

Comment 5

[Chris Peterson [:cpeterson]]

IIUC, James says this bug is about making about:reader pages appear in history (so Sync can see them), not about inserting arbitrary new history entries.

Comment 6

[Chris Peterson [:cpeterson]]

snorp says Christian is working around this Reader View issue with an extension page.

https://github.com/mozilla-mobile/android-components/issues/2879

Comment 7

[Chris Peterson [:cpeterson]]

Christian says using an extension page will improve reader view and also make navigation work. However, the link will be moz-extension://[id]/readerview.html?url= and not about:reader?url=. We should leave this bug open until we understand the implications of this for Sync (desktop and Fennec use of about:reader).

See also bug 1322304 about vanity URLs for extension pages.

Comment 8

[Chris Peterson [:cpeterson]]

Dylan recommends that this bug not block Fenix MVP. He'll talk to Christian about displaying the moz-extension: URLs in the address bar.

Comment 9

[Dylan Roeh (:droeh) (he/him)]

Eugen, David suggested you might have some insight here: I was thinking of approaching this by using nsISHistory.replaceEntry(), but that takes not just a replacement URL but a replace nsISHEntry that includes principal info. I'm concerned that replacing the URL without changing the principal is a possible security issue -- eg if we take a history entry that's loaded with a privileged principal and replace the URL only, we could end up loading arbitrary content with a privileged principal. Any thoughts on this? Is just using a null principal in the replacement nsISHEntry a reasonable approach?

Comment 10

[[ex-Mozilla] Agi Sferro | :agi]

Note that moz-extension: URLs have a special principal (the Extension principal).

Comment 11

[Eugen Sawin [:esawin]]

(In reply to Chris Peterson [:cpeterson] from comment #5)

IIUC, James says this bug is about making about:reader pages appear in history (so Sync can see them), not about inserting arbitrary new history entries.

(In reply to Dylan Roeh (:droeh) (he/him) from comment #9)

Eugen, David suggested you might have some insight here: I was thinking of approaching this by using nsISHistory.replaceEntry(), but that takes not just a replacement URL but a replace nsISHEntry that includes principal info. I'm concerned that replacing the URL without changing the principal is a possible security issue -- eg if we take a history entry that's loaded with a privileged principal and replace the URL only, we could end up loading arbitrary content with a privileged principal. Any thoughts on this? Is just using a null principal in the replacement nsISHEntry a reasonable approach?

Reading the discussion here, it's not clear to me whether we're still trying to only add moz-extension URLs to history or provide a general history insertion API.

In the latter case, allowing URL replacement on an inherited principal would be a security risk.
In the former case, we should probably replace the history entry with the extension (or expanded?) principal going by Agi's comment.
I think this is moving in a territory that'll require a sec review.

Comment 12

[Dylan Roeh (:droeh) (he/him)]

(In reply to Eugen Sawin [:esawin] from comment #11)

(In reply to Chris Peterson [:cpeterson] from comment #5)

IIUC, James says this bug is about making about:reader pages appear in history (so Sync can see them), not about inserting arbitrary new history entries.

(In reply to Dylan Roeh (:droeh) (he/him) from comment #9)

Eugen, David suggested you might have some insight here: I was thinking of approaching this by using nsISHistory.replaceEntry(), but that takes not just a replacement URL but a replace nsISHEntry that includes principal info. I'm concerned that replacing the URL without changing the principal is a possible security issue -- eg if we take a history entry that's loaded with a privileged principal and replace the URL only, we could end up loading arbitrary content with a privileged principal. Any thoughts on this? Is just using a null principal in the replacement nsISHEntry a reasonable approach?

Reading the discussion here, it's not clear to me whether we're still trying to only add moz-extension URLs to history or provide a general history insertion API.

In the latter case, allowing URL replacement on an inherited principal would be a security risk.
In the former case, we should probably replace the history entry with the extension (or expanded?) principal going by Agi's comment.
I think this is moving in a territory that'll require a sec review.

My thought was not to have a general history insertion API, but a history replacement API -- ie, you specify an entry in history (via index) and a replacement URI, and that entry gets its URI replaced. So in this case, when a moz-extension://[id]/readerview.html?url=foo URI gets loaded, the app can choose to replace it with about:reader?url=foo in history.

Also, to clarify: I thought moz-extension: URIs were already in history -- am I misunderstanding that?

Comment 13

[[ex-Mozilla] Agi Sferro | :agi]

Also, to clarify: I thought moz-extension: URIs were already in history -- am I misunderstanding that?

I think right now they're not but it's most likely because of Bug 1554302. I'll create a new bug if not -- either way replacing is what we want I agree.

Comment 14

[Eugen Sawin [:esawin]]

Sorry, insertion was the wrong term, but I'm reading from snorp's comment that we don't want a general API but a specific fix only for reader mode URLs. Have we changed course?

Comment 15

[Dylan Roeh (:droeh) (he/him)]

(In reply to Eugen Sawin [:esawin] from comment #14)

Sorry, insertion was the wrong term, but I'm reading from snorp's comment that we don't want a general API but a specific fix only for reader mode URLs. Have we changed course?

So, my understanding is: with extension pages (and with Agi's work on bug 1554302), the local usage story for this is fine -- the app can just prettify the URI when displaying it and leave the moz-extension: URI in session history. But for syncing this is problematic, as the id contained in moz-extension: URIs is not stable across devices. If we want to special case about:reader I can take a look at the approaches given in comment 4 instead, but it wasn't immediately clear to me whether this was decided to be a one-off situation or a bigger issue re: vanity URIs.

Comment 16

[Chris Peterson [:cpeterson]]

Deferring this bug from Fenix's M7 (July) milestone to the M8 backlog for later in Q3.

Comment 17

[Chris Peterson [:cpeterson]]

Dylan says the Reader View extension has a workaround, but it might cause problems with Sync on other devices.

not Q3

Comment 18

[Chris Peterson [:cpeterson]]

Tasks and enhancements should have severity N/A.