Closed Bug 1454262 Opened 6 years ago Closed 6 years ago

Regression: Search Bar highlight via cursor down/up keys is no longer possible

Categories

(Firefox :: New Tab Page, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 61
Iteration:
61.4 - May 7
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- unaffected
firefox61 + verified

People

(Reporter: mehmet.sahin, Assigned: rrosario)

References

Details

(Keywords: regression)

Attachments

(1 file)

Nightly 61.0a1 (2018-04-15) (64-Bit)
macOS 10.12.6

(1) Open a NTP
(2) Focus the Search Bar and type something so that the menu with the results appears
(3) Press on the arrow down/up keys to cycle through the search results

Actual: It is no longer possible to use the down/up keys.

Expected: It should be possible to use down/up keys.

This is a recent regression in latest Nightly.
It appears something in this merge broke it:
https://hg.mozilla.org/mozilla-central/rev/37b8862d354e

If I check out the previous merge, the up and down keys work:
https://hg.mozilla.org/mozilla-central/rev/a3df33c5d526

I don't see any of our changes in that merge. Or anything obvious that would cause it to break?
This was regressed by bug 1440189. Although sounds like this is something search should fix to not use keypress:

https://searchfox.org/mozilla-central/rev/bd326a2a6b729dc62a5aee57354a97ceac4d1dc0/browser/base/content/contentSearchUI.js#302-344
Blocks: 1440189
Keywords: regression
Assignee: nobody → rrosario
Iteration: --- → 61.3 - Apr 23
Priority: -- → P1
Thanks for fixing this one in NTP. We'd really love to keep the fix for the P1 webcompat bug 1440189.
Other keys, such as event.DOM_VK_TAB, event.DOM_VK_RIGHT, event.DOM_VK_DELETE and event.DOM_VK_ESCAPE are should be handled by keydown event. And perhaps, event.DOM_VK_RETURN too. Enter key keeps causing keypress event but when there is no consumer of Enter keypress event before listener, you can handle it at keydown event. Otherwise, you need to use system group event listener with nsIEventListenerManager.addSystemEventListener() <https://searchfox.org/mozilla-central/search?q=addSystemEventListener&case=false&regexp=false&path=>.
Ricky, can we expect a fix for this bug on your side or should we backout bug 1440189? The functionality is currently broken for our Nightly population. Thanks
Flags: needinfo?(rrosario)
(In reply to Pascal Chevrel:pascalc from comment #6)
> Ricky, can we expect a fix for this bug on your side

I'll have a patch for this early this coming week.
Flags: needinfo?(rrosario)
Severity: normal → critical
Iteration: 61.3 - Apr 23 → 61.4 - May 7
Comment on attachment 8970262 [details]
Bug 1454262 - Regression: Search Bar highlight via cursor down/up keys is no longer possible

https://reviewboard.mozilla.org/r/239054/#review244700

::: browser/base/content/contentSearchUI.js:54
(Diff revision 1)
>    this.input.autocomplete = "off";
>    this.input.setAttribute("aria-autocomplete", "true");
>    this.input.setAttribute("aria-controls", tableID);
>    tableParent.appendChild(this._makeTable(tableID));
>  
> -  this.input.addEventListener("keypress", this);
> +  this.input.addEventListener("keydown", this);

I ran `./mach mochitest browser/base/content/test/general/browser_contentSearchUI.js` and manually tested up/down arrow and tabbing through the dropdown.
Comment on attachment 8970262 [details]
Bug 1454262 - Regression: Search Bar highlight via cursor down/up keys is no longer possible

https://reviewboard.mozilla.org/r/239054/#review244704

Looks to work. Also tested with cmd-up/down for changing default engine, alt-up/down for changing one-time engine, cmd-shift-up/down for changing suggestions (when previously up/down to the one-time engine selection), tab, enter, delete (not backspace), etc. Soo many keyboard shortcuts ;)
Attachment #8970262 - Flags: review?(edilee) → review+
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/128dd159e3f5
Regression: Search Bar highlight via cursor down/up keys is no longer possible r=Mardak
https://hg.mozilla.org/mozilla-central/rev/128dd159e3f5
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Component: Activity Streams: Newtab → New Tab Page

I have verified that this issue is no longer reproducible using Firefox Nightly 61.0a1 (Build ID: 20180424220100) installed on Windows 10 x64. I can confirm that search results can be focused via the up/down arrows.

P.S. Sorry for the noise!

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.