Awesome bar pops up when click on form autocomplete fields

VERIFIED FIXED in fennec1.0b4

Status

--
blocker
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: aakashd, Assigned: vingtetun)

Tracking

Trunk
fennec1.0b4

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
Build Id:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2a2pre) Gecko/20090910
Fennec/1.0a3

and

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.3a1pre) Gecko/20090910
Fennec/1.0a3

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.2a2pre) Gecko/20090910
Fennec/1.0b4pre

Steps to Reproduce:
1. Go to www.gmail.com and wait for it to load and have the username field highlighted/focused
2. Click on the username field.

Actual Results:
The awesome bar will pop up 

Expected Results:
The awesome bar should not pop up.
This is not limited to gmail.

I saw this happen when I went to http://w3c.org, and click in the left top of page.
tracking-fennec: --- → ?
about:config does this as well for the checkbox and the let me in button.  no way to use about:config anymore.
Created attachment 400039 [details] [diff] [review]
Patch

This seems to work but need a bit more testing, though.
Created attachment 400363 [details] [diff] [review]
Patch

The same but expressed in an other manner and with clean up of (now) unneeded part of InputHandler.js
Attachment #400039 - Attachment is obsolete: true
Attachment #400363 - Flags: review?(mark.finkle)
Attachment #400363 - Flags: review?(froystig)
Attachment #400363 - Flags: review?(combee)

Updated

10 years ago
tracking-fennec: ? → 1.0+
Attachment #400363 - Flags: review?(froystig) → review-
Comment on attachment 400363 [details] [diff] [review]
Patch

>-  /**
>-   * Clear all recorded events.  This will *not* automagically redispatch any
>-   * pending un-redispatched events.  If you desire to redispatch everything
>-   * in the recorded events buffer, you should call _redispatchDownUpEvents()
>-   * before calling _clearDownUpEvents().
>-   *
>-   * @param [optional] the number of events to remove from the front of the
>-   * recorded events queue.
>-   */
>-  _clearDownUpEvents: function _clearDownUpEvents(howMany) {
>-    if (howMany === undefined)
>-      howMany = this._downUpEvents.length;
>-
>-    this._downUpEvents.splice(0, howMany);
>-    this._downUpDispatchedIndex = Math.max(this._downUpDispatchedIndex - howMany, 0);
>-  },

You want to keep this function, as you'll need to use it.  See following comment.


Suppose that this does *not* happen (i.e. the common case where commitToClicker is true):

>+    else
>       this._cleanClickBuffer();    // clean the click buffer ourselves, since there was no clicker
>                                    // to commit to.

Then the _commitAnotherClick in the if clause before this else clause will set a timeout (via setTimeout), which will eventually call _doSingleClick or _doDoubleClick, which will call _cleanClickBuffer on their own.  Within that timeout period, the InputHandler may already get a new MouseDown event, which will cause it to _recordEvent that event.

Now, _doSingleClick was doing

>-    this._cleanClickBuffer(2);
>+    this._cleanClickBuffer();

and _doDoubleClick was doing

>-    this._cleanClickBuffer(4);
>+    this._cleanClickBuffer();

because _cleanClickBuffer was taking that number as its howMany parameter and forwarding it to:

>-    this._clearDownUpEvents(howMany);

In place of which, this patch does:

>+    this._downUpEvents = [];

In the old case, howMany will make _clearDownUpEvents only splice out the events recorded in the MouseDown/MouseUp event group corresponding to this single/double click.  If you don't use _clearDownUpEvents with howMany, and instead just reset _downUpEvents to an empty array, you are also wiping out any events that may have been recorded during the timeout period started by _commitAnotherClick, which usually means you lose your MouseDown event.  Now your _downUpEvents array is smaller than expected, and MouseModule will throw array-item-undefined exceptions on the next time that something like:

>     let ev = this._downUpEvents[1].event;
>     this._clicker.singleClick(ev.clientX, ev.clientY);

is executed (similarly in the double-click case).

So that's a long explanation for the small detail as to why that function is still necessary and still needs to be called.  It's a tricky race condition.


Other changes look fine.
Created attachment 400449 [details] [diff] [review]
Patch v0.2

(In reply to comment #5)
> (From update of attachment 400363 [details] [diff] [review])

> So that's a long explanation for the small detail as to why that function is
> still necessary and still needs to be called.  It's a tricky race condition.

argh! :)
Attachment #400363 - Attachment is obsolete: true
Attachment #400449 - Flags: review?(mark.finkle)
Attachment #400449 - Flags: review?(froystig)
Attachment #400449 - Flags: review?(combee)
Attachment #400363 - Flags: review?(mark.finkle)
Attachment #400363 - Flags: review?(combee)
Comment on attachment 400449 [details] [diff] [review]
Patch v0.2

I'm glad to see that code simplified.
Attachment #400449 - Flags: review?(combee) → review+
Comment on attachment 400449 [details] [diff] [review]
Patch v0.2

Waiting for froystig to review before I push
Attachment #400449 - Flags: review?(mark.finkle) → review+
Attachment #400449 - Flags: review?(froystig) → review+
pushed:
https://hg.mozilla.org/mobile-browser/rev/7588e2aa35d4
Assignee: nobody → 21
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → B4
(Reporter)

Comment 10

10 years ago
verified FIXED on builds:

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2a2pre) Gecko/20090915 Fennec/1.0a3

and

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.3a1pre) Gecko/20090915
Fennec/1.0a3

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090915
Fennec/1.0b4pre
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.