1123424 - Reader Mode option does not appear after navigating back and forward

Status: RESOLVED DUPLICATE of bug 1122705

(Firefox for iOS :: Reader View, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

STR:

1) Visit a page that is compatible with Reader Mode
2) Enable reader mode
3) Navigate back
4) Navigate forward

Expected: The reader mode button should be there
Actual: The button is not there

This happens because we only trigger readability checks on first page load. When you go back/forward the page is cached and that code does not run again.

But the mozReadability results are actually still present in the cached page.

So instead we should update the Reader Mode status from WKNavigationDelegate.webView:didCommitNavigation:

(Which we did at one point, I think i removed it because it was not clear if that was actually needed)

Or, possibly better, is there a DOM event that we can listen to in ReaderMode.js, that fires when the page becomes active or visible? Then we can simply emit the usual userscriptmessage that we already have and not need any changes in the native side.

Comment 1

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Looks like this is relevant: https://developer.mozilla.org/en-US/docs/Web/Events/pageshow

"The pageshow event is fired when a session history entry is being traversed to. (This includes back/forward as well as initial page-showing after the onload event.)"

I think this should replace our global "mozCheckReadability();" that currently is in ReaderMode.js

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Yeah, IIRC we use "pageshow" for Reader on Android too. One piece of feedback that we get from time to time on Android is "Can the reader check be done sooner?" although this is usually on the initial network-fetch display of the page. People would like to see the Reader icon ASAP instead of waiting for the page to load and display. Just something to keep in mind.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Another potential tip: Watch out for multiple events being fired due to frames/iframes. On Android we do checks to make sure the event/notification is for the topmost DOM window.

Comment 4

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Yeah, IIRC we use "pageshow" for Reader on Android too. One piece of
> feedback that we get from time to time on Android is "Can the reader check
> be done sooner?" although this is usually on the initial network-fetch
> display of the page. People would like to see the Reader icon ASAP instead
> of waiting for the page to load and display. Just something to keep in mind.

I think this already happens on iOS because we trigger things on main frame load. As result the reader button appears almost instantly while the progress bar is still somewhere halfway.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

(In reply to Mark Finkle (:mfinkle) from comment #3)
> Another potential tip: Watch out for multiple events being fired due to
> frames/iframes. On Android we do checks to make sure the event/notification
> is for the topmost DOM window.

https://github.com/mozilla/firefox-ios/commit/4654bcde27ece0a4be56be073ec9707d125fb0a6

I'm not sure if this event actually fires for frames. There also doesn't seem to be a way to check that because the PageTransitionEvent does not contain any target info.

Comment 6

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Fix is part of https://github.com/mozilla/firefox-ios/pull/98