Closed Bug 1145217 Opened 5 years ago Closed 5 years ago

Disable "Add to reading list" button on about: pages (except about:reader)

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 39
Tracking Status
firefox38 --- verified
firefox39 --- verified

People

(Reporter: mcomella, Assigned: mcomella)

References

Details

Attachments

(1 file, 1 obsolete file)

Causes some issues as per bug 1127445 comment 72 (and down).

This is the quick fix to uplift - a proper fix would be to properly resolve about: page urls when these items are clicked from reader mode.
/r/5719 - Bug 1145217 - Disable toolbar add to reading list button on about: pages. r=margaret

Pull down this commit:

hg pull review -r aad27c20f17ec1710e47fae3820fa34974a64895
Attachment #8580134 - Flags: review?(margaret.leibovic)
Comment on attachment 8580134 [details]
MozReview Request: bz://1145217/mcomella

https://reviewboard.mozilla.org/r/5717/#review4675

Looks great. However, I think we should also file another bug to look more closely at our reading list code to make sure we can only add "valid" URLs. Looking at the logs that were posted in bug 1127445, it seems like the problem was that ReaderMode.jsm was running into an error trying to download content for these URLs. We should probably also make ReaderMode.jsm more defensive and not fall over in a case like this!
Attachment #8580134 - Flags: review?(margaret.leibovic) → review+
(In reply to :Margaret Leibovic from comment #2)
> Looks great. However, I think we should also file another bug

bug 1145217.
https://hg.mozilla.org/mozilla-central/rev/2a397413f85f
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment on attachment 8580134 [details]
MozReview Request: bz://1145217/mcomella

This should be uplifted with bug 1127445 - I will handle the uplift.

Approval Request Comment
[Feature/regressing bug #]: bug 1127445
[User impact if declined]:
  Readers can add about: pages to their reading list. When those items are selected, a "Loading..." screen appears and the page never loads. Users can still interact with the browser (e.g. go to a different url, open a new tab) so it isn't an ending interaction though.

[Describe test coverage new/current, TreeHerder]:
  None

[Risks and why]:
  Low - we disable a button on certain pages. Worst case - we disable the button on the wrong kind of pages.
 
[String/UUID change made/needed]: None
Attachment #8580134 - Flags: approval-mozilla-aurora?
"Add to reading list" button from the browser menu is disabled for about:home, about:firefox,  about:addons, about:apps, about:downloads, about:config, etc.
For about:reader I can't check due to bug Bug 1136157. Firefox crashes when going to about:reader
Attachment #8580134 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Tested with:
Device: Samsung S5 (Android 4.4)
Build: Firefox for Android 38.0a2 (2015-03-26)

The same behaviour as in Nightly: "Add to reading list" button from the browser menu is disabled for about:home, about:firefox,  about:addons, about:apps, about:downloads, about:config, etc.
But for about:reader I can't check due to bug Bug 1136157, which causes Firefox  to crash when going to about:reader
Based on Teo's comment will verify this bug as fixed. When Bug 1136157 will be fixed we will verify also the about:reader and we'll log a new bug if there's something wrong.  
Tested on Nightly 40.0a1 and Aurora 39.0a2
Status: RESOLVED → VERIFIED
Verified as fixed on Firefox 38 Beta 6
Attachment #8580134 - Attachment is obsolete: true
Attachment #8619816 - Flags: review+
You need to log in before you can comment on or make changes to this bug.