1127989 - Fixes to BackForwardListViewController

Status: RESOLVED FIXED

(Firefox for iOS :: Browser, defect)

Description

[Brian Nicholson (:bnicholson)]

Playing around with our back/forward list, I've run across a few problems:
* Entries in the list are empty if the page doesn't have a title
* Whenever long pressing the back/forward buttons, the following warning appears in the console:

2015-01-30 10:44:11.324 Client[36645:1389340] Warning: Attempt to present <Client.BackForwardListViewController: 0x7fe7b2279400> on <Client.BrowserViewController: 0x7fe7b2229130> whose view is not in the window hierarchy!

This happens because UIGestureRecognizer callbacks are fired multiple times in different states (Possible, Began, Changed, Ended, etc.), and we're firing the delegate multiple times. Whenever handling a UIGestureRecognizer event, we need to check its state. I have a feeling we're doing this wrong in other places we use UIGestureRecognizer, so I'll file more bugs for any I find.

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: Pull request

Comment 2

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Is the problem here that we are adding a gesturerecognizer on top of a button, which already has its own click handling?

Is that what you are seeing? That both the long press gesturerecognizer is fired, and also the normal button back/forward action on TouchUpInside?

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

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

Looks like checking the state for Began is a common way to do this when having a UILongPressGestureRcoginzier on a UIButton. Looks good.

Comment 4

[Brian Nicholson (:bnicholson)]

(In reply to Stefan Arentz [:st3fan] from comment #2)
> Is the problem here that we are adding a gesturerecognizer on top of a
> button, which already has its own click handling?
> 
> Is that what you are seeing? That both the long press gesturerecognizer is
> fired, and also the normal button back/forward action on TouchUpInside?

Nope, this isn't related to the fact that it's a button; the problem is that the gesture recognizer callback is called multiple times for multiple states. From what I can tell, a gesture recognizer callback will always be called at least twice: once for the Began state, and once for the Ended state. It can be called an indefinite number of times between those for the Changed state, too, when dragging around.