firefox-ios icon indicating copy to clipboard operation
firefox-ios copied to clipboard

#9500: Add to Reading List is allowed on non-reader mode pages

Open q2r5 opened this issue 3 years ago • 4 comments

Fixes #9500.

Only check the reader mode state if the MIME type is HTML.

q2r5 avatar Nov 15 '21 20:11 q2r5

hi @q2r5 ! As always, it's great seeing your contributions 😄

With the suggested changes, it looks like we still get the option to add to reading list. At launch, I saw that it wasn't available. But when you search again, you'll see it.

nishant2718 avatar Nov 18 '21 21:11 nishant2718

The option to add to reading list is shown on search result pages because they can be converted to reader mode, both with these changes and on v95 (6631). As I understood it, the issue was for pages not available for reader mode.

q2r5 avatar Nov 19 '21 03:11 q2r5

I think what you're referring to is #9747 @nishant2718 ? I think those are two different issues, although similar.

lmarceau avatar Jan 14 '22 22:01 lmarceau

This pull request has conflicts when rebasing. Could you fix it @q2r5?

mergify[bot] avatar Sep 12 '22 21:09 mergify[bot]

@q2r5 I just reviewed and the fix seems to be great, but the variable readerModeAvailableOrActive seems to be not used anymore. This is certainly because this PR has been sitting here for too long, but sadly, this means the work doesn't fix the issue anymore. If you kindly make the required changes to make the fix work again I will review it asap. Thank you for your contribution!

yoanarios avatar Jan 31 '23 15:01 yoanarios

Green build!! Screen Shot 2023-02-01 at 9 47 19 AM

yoanarios avatar Feb 01 '23 14:02 yoanarios

LGTM, @q2r5 Thank you for your contribution!

yoanarios avatar Feb 01 '23 14:02 yoanarios