825924 - Form autocomplete does not properly reset

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[ael]

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121128204232

Steps to reproduce:

- Double click in a form field (type text mainly) to display the autocomplete history
- Select an item
- Select the field content, right-click and cut
- Double click again in the form field


Actual results:

- The autocomplete history only shows the last selected item


Expected results:

- The autocomplete history should display all the history

This only happens when the field content is deleted via mouse selection and cutting. When deleting with the keyboard the history is correctly shown.

Switching tab before double clicking in the field again also reset the history correctly.

Comment 1

[Alice0775 White]

Regression window(cached m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/c3cb87871829
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120325 Firefox/14.0a1 ID:20120326174026
Bad:
http://hg.mozilla.org/mozilla-central/rev/0ff816e5e992
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120327 Firefox/14.0a1 ID:20120327100541
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c3cb87871829&tochange=0ff816e5e992

Regression window(cached m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/90f3a9a6e197
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120326 Firefox/14.0a1 ID:20120326174729
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/3f9b7201c29d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120326 Firefox/14.0a1 ID:20120326195131
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=90f3a9a6e197&tochange=3f9b7201c29d

Suspected: Bug 668606

Comment 2

[(no longer active)]

Masayuki, can you please take a look at this?  Thanks!

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

The cause is nsEditor fires untrusted input event if the edit is caused by command. Of course, it's a bug.

However, according to the document of D3E spec,
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#trusted-events
Shouldn't we dispatch each input event as trusted event even if the change is caused by untrusted event? Even if the changed cause is untrusted event, the change actually happens and the input event is the notification. So, I think that nsEditor should always dispatch trusted input events. How do you think, Smaug?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Yeah, I think if there is some "input" to editor, the event needs to be trusted.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Thank you. I'll write such patch.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch

This patch make nsEditor always dispatch trusted input events. Then, autocomplete handler always handle properly.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=c509d2945a6e

Comment 8

[(no longer active)]

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

Review of attachment 700963 [details] [diff] [review]:
-----------------------------------------------------------------

Hmm OK.  Make sure that this doesn't break any tests though, IIRC we had tests which did expect non-trusted input events at some point.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> Hmm OK.  Make sure that this doesn't break any tests though, IIRC we had
> tests which did expect non-trusted input events at some point.

Yep, on tryserver, all green. Fortunately, we don't accept untrusted key events now. So, I guess that there is no case we dispatch untrusted input event except unintentionally such as this bug.

Comment 10

[(no longer active)]

(In reply to comment #9)
> > Hmm OK.  Make sure that this doesn't break any tests though, IIRC we had
> > tests which did expect non-trusted input events at some point.
> 
> Yep, on tryserver, all green. Fortunately, we don't accept untrusted key events
> now. So, I guess that there is no case we dispatch untrusted input event except
> unintentionally such as this bug.

Hmm, weird.  But if the try server is happy, then who am I to disagree? :-)

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to :Ehsan Akhgari from comment #10)
> (In reply to comment #9)
> > > Hmm OK.  Make sure that this doesn't break any tests though, IIRC we had
> > > tests which did expect non-trusted input events at some point.
> > 
> > Yep, on tryserver, all green. Fortunately, we don't accept untrusted key events
> > now. So, I guess that there is no case we dispatch untrusted input event except
> > unintentionally such as this bug.
> 
> Hmm, weird.  But if the try server is happy, then who am I to disagree? :-)

I guess that this is what you worry about. The change refuses any change by untrusted key events, therefore, basically, untrusted input events shouldn't be fired from editor.
https://bugzilla.mozilla.org/show_bug.cgi?id=698949

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/46c4522fee6b

Comment 13

[(no longer active)]

(In reply to comment #11)
> (In reply to :Ehsan Akhgari from comment #10)
> > (In reply to comment #9)
> > > > Hmm OK.  Make sure that this doesn't break any tests though, IIRC we had
> > > > tests which did expect non-trusted input events at some point.
> > > 
> > > Yep, on tryserver, all green. Fortunately, we don't accept untrusted key events
> > > now. So, I guess that there is no case we dispatch untrusted input event except
> > > unintentionally such as this bug.
> > 
> > Hmm, weird.  But if the try server is happy, then who am I to disagree? :-)
> 
> I guess that this is what you worry about. The change refuses any change by
> untrusted key events, therefore, basically, untrusted input events shouldn't be
> fired from editor.
> https://bugzilla.mozilla.org/show_bug.cgi?id=698949

Yes, nice find!  Thanks!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/46c4522fee6b

Comment 15

[ael]

The latest nightly works as expected for the few test cases where I noticed this problem. Thanks a lot for the quick fix.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to ael from comment #15)
> The latest nightly works as expected for the few test cases where I noticed
> this problem. Thanks a lot for the quick fix.

Thank you for your verification.

-> v.