Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Add and process reader view history entries #2879

Closed
csadilek opened this issue Apr 30, 2019 · 7 comments
Closed

Add and process reader view history entries #2879

csadilek opened this issue Apr 30, 2019 · 7 comments
Assignees
Labels
🌟 feature New functionality and improvements needs:gv To implement/fix this we need a new API in GeckoView 🔎 Needs investigation P2 <readerview> feature-readerview <web-extensions>

Comments

@csadilek
Copy link
Contributor

csadilek commented Apr 30, 2019

After #2624 lands, we still need to add a history entry (about:reader...) when reader view is activated and add functionality to process it when navigating.

┆Issue is synchronized with this Jira Task

@csadilek csadilek added 🌟 feature New functionality and improvements <readerview> feature-readerview labels Apr 30, 2019
@csadilek csadilek changed the title Add and process reader mode history entries Add and process reader view history entries May 3, 2019
@csadilek csadilek self-assigned this May 7, 2019
@csadilek
Copy link
Contributor Author

csadilek commented May 8, 2019

We currently can't implement this without either new GV API or some web extension enhancements. Discussion continues in: https://bugzilla.mozilla.org/show_bug.cgi?id=1550144

@csadilek csadilek added the needs:gv To implement/fix this we need a new API in GeckoView label May 8, 2019
@cpeterso
Copy link
Contributor

We currently can't implement this without either new GV API or some web extension enhancements. Discussion continues in: https://bugzilla.mozilla.org/show_bug.cgi?id=1550144

@csadilek - snorp says you are working around this Reader View issue with an extension page. Is that correct or do you still need a new GV API or extension enhancements?

@csadilek
Copy link
Contributor Author

@cpeterso Yes, using an extension page will improve reader view lot and also make navigation work. However, the link will be moz-extension://[id]/readerview.html?url= and not about:reader?url=. So, I wanted to leave that ticket open until I understand the implications of this for sync (desktop and fennec use about:reader). There's also a relevant discussion here about allowing to make those extension page URLs prettier but that seems to have stalled: https://bugzilla.mozilla.org/show_bug.cgi?id=1322304

@csadilek
Copy link
Contributor Author

csadilek commented Jun 22, 2020

This is now also causing limitations in Fenix:

@mcarare
Copy link
Contributor

mcarare commented Jun 24, 2020

Can / should this be fixed before release?

@csadilek
Copy link
Contributor Author

csadilek commented Jun 24, 2020

@mcarare I think we can address the two Fenix issues by using the original URL instead, as you mentioned. We don't have to extract (or parse) it, as it's already available in the browser store via store.state.findTab(session.id)?.readerState.activeUrl. Example: https://github.com/mozilla-mobile/fenix/blob/fee09c3092d037315d4403cc98eabf3523a8e02c/app/src/main/java/org/mozilla/fenix/ext/Session.kt#L23

@agi90 and I talked through options how to implement support for about:reader, or URL mapping functionality for extensions in general. Theoretically, this can either be done in Gecko (https://bugzilla.mozilla.org/show_bug.cgi?id=1322304), GeckoView (https://bugzilla.mozilla.org/show_bug.cgi?id=1550144) or in A-C directly. I think we need to decide how to implement this first. So, we likely need the short-term fix above.

@csadilek
Copy link
Contributor Author

csadilek commented Jun 24, 2020

@agi90 pointed me to protocol headers https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/protocol_handlers which should be able to provide what we want e.g. a ext+reader:// url.

For sharing in Fenix, we would still want to use the original URL, as the receiver might not have ext+reader.

@csadilek csadilek self-assigned this Jun 24, 2020
@data-sync-user data-sync-user changed the title Add and process reader view history entries FNX2-16483 ⁃ Add and process reader view history entries Aug 1, 2020
@data-sync-user data-sync-user changed the title FNX2-16483 ⁃ Add and process reader view history entries FNX3-22289 ⁃ Add and process reader view history entries Aug 1, 2020
@st3fan st3fan changed the title FNX3-22289 ⁃ Add and process reader view history entries Add and process reader view history entries Aug 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🌟 feature New functionality and improvements needs:gv To implement/fix this we need a new API in GeckoView 🔎 Needs investigation P2 <readerview> feature-readerview <web-extensions>
Projects
None yet
Development

No branches or pull requests

5 participants