1449018 - Address bar -> Bookmark editor -> Tags: text box -> Tags: auto-completion feature is broken

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[CKOTTMozillaBugzilla]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180315233128

Steps to reproduce:

*Given: (3) tags that begin with an uppercase 'A' are defined: 'Apple', 'Avery', 'Azure'

In address bar:
- Click on the star icon to open the bookmark editor
- Click on the Tags: text box
- Type a lowercase 'a'
- - - Notice: 'apple', with a lowercase 'a', is copied to the Tags: text box
- Press down arrow three times, to highlight 'Azure' in the Tags: autocomplete dropdown
- - - Notice: The Tags: text box still contains 'apple'
- Press Enter



Actual results:

- The bookmark editor immediately disappears
- The bookmark is now tagged with 'apple', with a lowercase 'a'
- The previously-defined tag named 'Apple' (with an uppercase 'A') has been changed to 'apple' (with a lowercase 'a')



Expected results:

- The bookmark editor remains open
- The Tags: autocomplete dropdown disappears
- 'Azure', with an uppercase 'A', is copied to the Tags: text box

(this is the behavior of version 59.x and prior versions)

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:the tag system is broken.

Comment 2

[Marco Bonardo [:mak]]

This is probably due to moving from tree autocomplete to richlistbox autocomplete, the behavior is surely slightly different, we must check it out and see what we can do. Anyway we are working on a tags refactoring for 61, I can't tell if we can easily fix this in 60, it's likely it may need autocomplete changes that may be scary for an uplift.
For now to complete you can use the Right arrow key.

Comment 3

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8fa7c8455f707d4fa36a01469ae24af6ad0cde02&tochange=c0b4ca69376c1b8cd1c22a202100363631b5b9a6

Regressed by;
c0b4ca69376c	Masayuki Nakano — Bug 1434837 - Make autocomplete and satchel listen to keypress event at the system event group r=mak

@:masayuki
Your patch seems to cause the regression, can you look into this?

Comment 4

[Marco Bonardo [:mak]]

thanks for checking

Comment 5

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=81edf404c07501d73bd2060031b7175f091d53a9

Comment 6

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5505d866abf2e357faabd39343207fe7eeebab17

Comment 7

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

Attachment: Bug 1449018 - The bookmark panel should listen to keypress event at the system event group

The bookmark panel should handle keypress events at the system event group
because it needs to handle keypress events after autocomplete module which
handles keypress events at the system event group.

Review commit: https://reviewboard.mozilla.org/r/231508/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231508/

Comment 8

[Marco Bonardo [:mak]]

Comment on attachment 8962699 [details]
Bug 1449018 - The bookmark panel should listen to keypress event at the system event group

https://reviewboard.mozilla.org/r/231508/#review237442

Thank you, please ask for uplift approval once possible.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e9ff0123aa27
The bookmark panel should listen to keypress event at the system event group r=mak

Comment 10

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/e9ff0123aa27

Comment 11

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

Comment on attachment 8962699 [details]
Bug 1449018 - The bookmark panel should listen to keypress event at the system event group

Approval Request Comment
[Feature/Bug causing the regression]:
Regression of bug 1434837.

[User impact if declined]:
Cannot use autocomplete in the bookmark panel (no problem with property dialog of bookmark item).

[Is this code covered by automated tests?]:
Yes.

[Has the fix been verified in Nightly?]:
Yes.

[Needs manual test from QE? If yes, steps to reproduce]: 
No since tested by new automated tests.

[List of other uplifts needed for the feature/fix]:
No.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
Enter key and Escape key is handled by the bookmark panel directly. I.e., not changing common widget of toolkit or something.

[String changes made/needed]:
No.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8962699 [details]
Bug 1449018 - The bookmark panel should listen to keypress event at the system event group

one-line fix for bookmark panel, approved for 60.0b8

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/294c100c1a17