Closed Bug 536944 Opened 10 years ago Closed 10 years ago

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

Categories

(Firefox for Android Graveyard :: General, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: vingtetun, Assigned: vingtetun)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Patch (obsolete) — Splinter Review
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.
Attachment #419289 - Flags: review?(webapps)
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.
Attachment #419289 - Flags: review?(webapps) → review-
Attached patch Patch v0.2Splinter Review
(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
Assignee: nobody → 21
Attachment #419289 - Attachment is obsolete: true
Attachment #423786 - Flags: review?(webapps)
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?
Attachment #423786 - Flags: review?(webapps) → review+
(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.
Whiteboard: [fennec-checkin-post1.0]
pushed with comment change:
http://hg.mozilla.org/mobile-browser/rev/f25a704a665b
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fennec-checkin-post1.0]
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.