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)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla39
Tracking Status
firefox38 --- verified
firefox39 --- verified

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.
/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 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)
(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 on attachment 8582782 [details]
MozReview Request: bz://1147122/margaret

Ah, then r=me with the updated comment
Attachment #8582782 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/f4cfd1e29e8a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
Flags: qe-verify+
QA Contact: andrei.vaida
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
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?
Attachment #8582782 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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.
Attachment #8582782 - Attachment is obsolete: true
Attachment #8619864 - Flags: review+
Attachment #8619865 - Flags: review+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: