1230504 - Request Desktop Site should not be available in reader view

Status: RESOLVED FIXED

(Firefox for iOS :: Reader View, defect)

Description

[Catalin Suciu [:csuciu]]

1. Open an article in reader mode
2. Long tap on the refresh button

Result: RDS option pops up

Note: On Safari the refresh button is not available in reader mode

Comment 1

[Johannes Marbach]

Attachment: Pull request

This is a very small change that simply performs a standard reload when long-tapping the refresh button in reader mode instead of presenting the RDS/RMS action sheet / popover.

Comment 2

[Johannes Marbach]

Sorry, I just saw the note about Safari hiding the refresh button in reader mode. I could try to implement that instead of the simple reload approach in the current pull request. Just let me know what would be more desirable.

Comment 3

[Brian Nicholson (:bnicholson)]

Robin, what do you think? Disable the refresh button in reader mode, or allow it without the long-press?

Comment 4

[Robin Andersen [:tecgirl]]

Disable the long-press for RDS, but keep refresh.

Comment 5

[Johannes Marbach]

Would it be acceptable to just let the long-tap event handler return without doing anything in reader mode? When you long-tap the button in that case it'll highlight in blue until you either release it or the gesture recognizer times out which, I believe, is after 6 seconds. That would be a very easy fix but the UX feedback might be a bit confusing.

If we want to truly disable long-tapping in reader mode, we'd need to expose the gesture recognizer to the browser view controller and toggle it on and off when the user switches into and out of reader mode.

Comment 6

[Brian Nicholson (:bnicholson)]

(In reply to hennes from comment #5)
> Would it be acceptable to just let the long-tap event handler return without
> doing anything in reader mode?

I think this will be OK -- I think this bug is a pretty unusual edge case, so let's just go with the easy fix for now.

Comment 7

[Johannes Marbach]

(In reply to Brian Nicholson (:bnicholson) from comment #6)
> I think this will be OK -- I think this bug is a pretty unusual edge case,
> so let's just go with the easy fix for now.

Cool, this boils down to a mini-change then. PR is updated accordingly. :)

Comment 8

[Brian Nicholson (:bnicholson)]

Comment on attachment 8696024 [details] [review]
Pull request

Looks great. Thanks!

Comment 9

[Brian Nicholson (:bnicholson)]

https://github.com/mozilla/firefox-ios/commit/002b9e2828481c108b9f70b9931634bb4501a30a