536944 - this._clicker is null error in InputHandler.js#doSingleClick

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

When there is a click on the content and then a click in the chrome less than 400ms after, InputHandler throw an error because this._clicker has been set to null by the mousedown on the chrome UI.
In my opinion if a user quickly click on the content and then go to the chrome in less than 400ms we probably don't want a click to happen.

Comment 1

[Benjamin Stover (:stechz)]

Comment on attachment 419289 [details] [diff] [review]
Patch

Instead of calling clearTimeout, probably should call cleanClickBuffer so the first click isn't left hanging about.

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v0.2

(In reply to comment #1)
> (From update of attachment 419289 [details] [diff] [review])
> Instead of calling clearTimeout, probably should call cleanClickBuffer so the
> first click isn't left hanging about.


You was right, but we also need to call clearTimeout to prevent a click to be fired

Comment 3

[Benjamin Stover (:stechz)]

Comment on attachment 423786 [details] [diff] [review]
Patch v0.2

>+      if (this._clickTimeout) { // it stays click(s) from content
>+        window.clearTimeout(this._clickTimeout);
>+        this._cleanClickBuffer();
>+      }

Put comment on next line.  Could be written better: "cancel all pending content clicks" perhaps?

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to comment #3)
> (From update of attachment 423786 [details] [diff] [review])
> >+      if (this._clickTimeout) { // it stays click(s) from content
> >+        window.clearTimeout(this._clickTimeout);
> >+        this._cleanClickBuffer();
> >+      }
> 
> Put comment on next line.  Could be written better: "cancel all pending content
> clicks" perhaps?

it should be better :)
If the (future) lander of the patch can change that when landing it would be nice.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed with comment change:
http://hg.mozilla.org/mobile-browser/rev/f25a704a665b

Comment 6

[Aakash Desai [:aakashd]]

verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.2pre) Gecko/20100217 Namoroka/3.6.2pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a2pre) Gecko/20100217 Namoroka/3.7a2pre Fennec/1.1a2pre