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

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: bz://1145217/mcomella

/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

Comment 2

[: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!

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to :Margaret Leibovic from comment #2)
> Looks great. However, I think we should also file another bug

bug 1145217.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/2a397413f85f

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2a397413f85f

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

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

Comment 7

[Teodora Vermesan (:TeoVermesan)]

"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

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e1a108384863

Comment 9

[Teodora Vermesan (:TeoVermesan)]

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

Comment 10

[Catalin Suciu [:csuciu]]

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

Comment 11

[u421692]

Verified as fixed on Firefox 38 Beta 6

Comment 13

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: MozReview Request: Bug 1145217 - Disable toolbar add to reading list button on about: pages. r=margaret