1331981 - Reader mode displays "Failed to load article from page" when reloading this page

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect, P4)

Description

[Markus Stange [:mstange]]

Steps to reproduce:
 1. Go to https://hg.mozilla.org/mozilla-central/raw-file/6a23526fe5168087d7e4132c0705aefcaed5f571/toolkit/components/perfmonitoring/README.md
 2. Switch into reader mode.
 3. Click the reload button in the URL bar.

Expected results: The page should look the same as before the reload.
Actual results: The page is replaced by the text "Failed to load article from page".

The same happens on session restore.

Comment 1

[Evan Tseng [:evanxd]]

Interesting, this issues cannot reproduce on every page. I can reproduce with the page listed on Comment 0, but cannot with this page[1].

Need to investigate deeper...

[1]: https://us.yahoo.com/tech/one-big-reason-not-buy-140400301.html

Comment 2

[psn]

64-bit Nightly build 55 in Windows 8.1.
I see this "Failed to load article from page" on both given links; and on other pages too: Wikipedia for one..
Pressing F5 loads the page in Reader View.

Comment 3

[:Gijs (he/him)]

Looks like this is because the page in comment 0 is a text/plain document . When we load the page initially, we can use the DOM from the page, but when we refresh we try to fetch the document using XHR and as a result we then get the text/plain thing, which isn't a (DOM) document and so reader mode fails to work.

The fix would involve detecting the text/plain case when we retrieve the document using XHR. I'm fairly sure that pretty much all the logic for creating a DOM in the browser for text documents is just sticking all the content in a <pre>. We could probably try to do better (like figuring out paragraphs, and then dumping that directly into the result document without using Readability directly), especially because reader mode can't really do anything unless you give it a reasonable DOM.

That said, rendering plaintext documents into reader mode isn't really its core mission, so P4 still seems about right to me.

Comment 4

[kabakert]

Attachment: WIP: Bug 1331981 - WIP commit for review. r=niklas,mtigley

Comment 5

[kabakert]

Attachment: Bug 1331981 - Fixed Refresh in Reader Mode for text/plain documents. r=niklas,mtigley

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c895070ae91d
Fixed Refresh in Reader Mode for text/plain documents. r=mtigley

Comment 7

[Norisz Fay [:noriszfay]]

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