1124900 - add test coverage for the keyboard navigation in the new search panel

Status: RESOLVED FIXED

(Firefox :: Search, enhancement)

Description

[Florian Quèze [:florian]]

The handling of keyboard events in the new search panel is complicated and would be easy to regress when changing it, so I think it needs tests.

Comment 1

[Florian Quèze [:florian]]

Attachment: Patch

The only reason why I'm requesting feedback rather than review is that try server shows this as making the browser_yahoo_behavior.js test as failing on Linux (https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6c945046e6f) and I haven't investigated why yet.

Comment 2

[Dave Townsend [:mossop]]

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

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

::: browser/components/search/test/browser_searchbar_keyboard_navigation.js
@@ +6,5 @@
> +
> +const kValues = ["foo1", "foo2", "foo3"];
> +const kUserValue = "foo";
> +
> +function promiseNewEngine(basename) {

Maybe we should move this into head.js

Comment 3

[Florian Quèze [:florian]]

Attachment: Patch v2

Addressed comment 2. Now requesting review, as I'm fixing the Orange I mentioned in comment 1 in bug 1126816.

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8558018 [details] [diff] [review]
Patch v2

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

::: browser/components/search/test/browser_searchbar_openpopup.js
@@ -50,5 @@
> -      }
> -    });
> -  });
> -}
> -

You can also remove this from browser_searchSuggestionUI.js and browser_searchHighlight.js I think

Comment 5

[Florian Quèze [:florian]]

Attachment: Patch v2, unbitrotted

(In reply to Dave Townsend [:mossop] from comment #4)

> You can also remove this from browser_searchSuggestionUI.js and
> browser_searchHighlight.js I think

No, these are in the browser/base/content/test/general folder rather than browser/components/search/test for some reason.

Comment 6

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/365129a004d7

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/365129a004d7

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/86222ca4b309