Closed Bug 1146666 Opened 5 years ago Closed 5 years ago

reader view available for direct load but not via homepage of @medium

Categories

(Toolkit :: Reader Mode, defect)

defect
Not set
Points:
3

Tracking

()

VERIFIED FIXED
mozilla39
Iteration:
39.3 - 30 Mar
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: clarkbw, Assigned: Gijs)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

The STR might not work once this article is no longer on the homepage.
https://medium.com/@lukek/an-eleven-year-old-s-guide-to-plane-spotting-53f8dcea3ec9

Directly going to that article shows it available for reader view.  However if you go to the homepage of medium https://medium.com/ and navigate to the article it won't be available for reader view.
I can confirm that this issue affects both Nightly 39.0a1 (2015-03-23) and Aurora 38.0a2 (2015-03-23).
Flags: qe-verify+
I'm 90% sure that this is because Medium isn't actually navigating to the article. It's just doing an XHR and updating the contents (ala a single-page webapp). But the URL is changing, so maybe pushState is being used?

This is likely to be a generic issue with any clever website that defers loading their actual content until after the actual initial page load. Not sure if there's a any good way to handle that (mutation observers? polling?).

At least with pushState there's the potential for using that as a signal to recheck the page... Although a quick check of devmo indicates there's only a onpopstate event but nothing for onpushstate. So we might need a chrome-only hack for that.

Note that this bug also exists in the reverse -- if you load a Medium article, scroll to the very bottom, and click the "More stories on Medium >" button, you'll get an XHR load of the homepage but the reader button remains.
On the bright side, now that we delay parsing the page until you click the button, we will actually be parsing what the user sees.

Do we get a location change event when a webpage updates the URL with the pushState API?
(In reply to :Margaret Leibovic from comment #3)
> On the bright side, now that we delay parsing the page until you click the
> button, we will actually be parsing what the user sees.
> 
> Do we get a location change event when a webpage updates the URL with the
> pushState API?

I just checked with a breakpoint on Firefox 37, seems like "no"...
Anton, can you help out here from the Medium side?
Flags: needinfo?(anton)
... but the docs on nsIWebProgressListener imply it *should*!
The breakpoint lied. :-\

Going to just take this and see if I can work out how to fix.
Status: NEW → ASSIGNED
Flags: needinfo?(anton)
Iteration: --- → 39.3 - 30 Mar
Points: --- → 3
Flags: in-testsuite-
Flags: firefox-backlog+
Assignee: nobody → gijskruitbosch+bugs
Attached file MozReview Request: bz://1146666/Gijs (obsolete) —
/r/6015 - Bug 1146666 - fix reader mode button's dealing with history.pushState, r?margaret

Pull down this commit:

hg pull review -r a73e9d5082371e0a4eadbbb1af2aa155927f39b6
Attachment #8582787 - Flags: review?(margaret.leibovic)
This issue likely affects mobile as well, although the manifestation would be different because of our parse-on-load logic (we would successfully show an article when you click the button, but it would be an old page, not what you're currently looking at). I can file a separate bug for this, although I'm planning to make our button logic consistent with desktop, so I can just include parallel browser.js/content.js changes in my patch to do that.
Blocks: 1132074
Comment on attachment 8582787 [details]
MozReview Request: bz://1146666/Gijs

https://reviewboard.mozilla.org/r/6013/#review4973

::: browser/base/content/browser.js
(Diff revision 1)
> -    if (aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)
> +    if (aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT) {

Nice find. It's handy that this already existed!
Attachment #8582787 - Flags: review?(margaret.leibovic) → review+
https://hg.mozilla.org/mozilla-central/rev/e3319e722309
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Verified fixed on Nightly 39.0a1 (2015-03-29) and Aurora 38.0a2 (2015-03-29), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
QA Contact: andrei.vaida
Attachment #8582787 - Attachment is obsolete: true
Attachment #8619856 - Flags: review+
Depends on: 1445337
Depends on: 1445351
You need to log in before you can comment on or make changes to this bug.