Closed
Bug 1147122
Opened 9 years ago
Closed 9 years ago
Restore reader view error message if about:reader fails when user clicks reader button
Categories
(Toolkit :: Reader Mode, defect)
Toolkit
Reader Mode
Tracking
()
VERIFIED
FIXED
mozilla39
People
(Reporter: Margaret, Assigned: Margaret)
References
Details
Attachments
(2 files, 1 obsolete file)
Right now, if we fail to find an article to load in about:reader, we just fall back to loading the original URL directly. This makes sense if we're trying to load an about:url directly (i.e. what we do from the reading list on Android). But if the user clicks on the toolbar button and we fail, reloading the original page is confusing. We already have an error message string in the tree from when we used to show an error on Android: http://mxr.mozilla.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/aboutReader.properties#6 I think we should just show an error message with this string in the (hopefully unlikely) case that the user hits the button and we fail. In the future, we can make this error more robust, with things like a feedback link, but not for Fx38.
Assignee | ||
Comment 1•9 years ago
|
||
/r/6007 - Bug 1147122 - Restore reader view error message if about:reader fails when user clicks reader button Pull down this commit: hg pull review -r 37167ed99a51a62defe7aab4743d0be55ea01786
Attachment #8582782 -
Flags: review?(gijskruitbosch+bugs)
Comment 2•9 years ago
|
||
Comment on attachment 8582782 [details] MozReview Request: bz://1147122/margaret https://reviewboard.mozilla.org/r/6005/#review4969 ::: toolkit/components/reader/AboutReader.jsm (Diff revision 1) > - if (this._article || this._windowUnloaded) { > + if (this._article || this._windowUnloaded || this._error) { Nit: Please update the comment above this. ::: toolkit/components/reader/AboutReader.jsm (Diff revision 1) > + // Otherwise, just load the original URL. What's the otherwise case here?
Attachment #8582782 -
Flags: review?(gijskruitbosch+bugs)
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to :Gijs Kruitbosch from comment #2) > ::: toolkit/components/reader/AboutReader.jsm > (Diff revision 1) > > + // Otherwise, just load the original URL. > > What's the otherwise case here? This is the case where we're trying to load the about:reader URL directly (e.g. loading a reading list item on Fennec, session restore).
Comment 4•9 years ago
|
||
Comment on attachment 8582782 [details]
MozReview Request: bz://1147122/margaret
Ah, then r=me with the updated comment
Attachment #8582782 -
Flags: review+
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/f4cfd1e29e8a
status-firefox38:
--- → affected
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f4cfd1e29e8a
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Updated•9 years ago
|
Flags: qe-verify+
QA Contact: andrei.vaida
Comment 7•9 years ago
|
||
Verified fixed on Aurora 39.0a2 (2015-04-02), using Ubuntu 14.04 (x64), Windows 7 (x64) and Mac OS X 10.9.5. The following error message is now displayed in Reader View for pages that could not be loaded: "Failed to load article from page".
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8582782 [details]
MozReview Request: bz://1147122/margaret
Approval Request Comment
[Feature/regressing bug #]: reader view
[User impact if declined]: about:reader will redirect to original URL if there was an error showing reader view, instead of showing an error message
[Describe test coverage new/current, TreeHerder]: landed on nightly/aurora
[Risks and why]: low-risk, restores some error message logic that we used to ship in Fennec (but never actually removed)
[String/UUID change made/needed]: none
Attachment #8582782 -
Flags: approval-mozilla-beta?
Updated•9 years ago
|
Attachment #8582782 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•9 years ago
|
||
Verified fixed on Beta 38.0b2-build1 (20150406174117) as well, using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8582782 -
Attachment is obsolete: true
Attachment #8619864 -
Flags: review+
Attachment #8619865 -
Flags: review+
Assignee | ||
Comment 12•9 years ago
|
||
Assignee | ||
Comment 13•9 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•