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

RESOLVED FIXED in Firefox 37

Status

()

enhancement
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Trunk
Firefox 38
x86
All
Points:
3
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox37 fixed, firefox38 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Posted patch Patch (obsolete) — Splinter Review
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.
Assignee: nobody → florian
Status: NEW → ASSIGNED
Attachment #8553367 - Flags: feedback?(dtownsend)
Blocks: 1124904
Points: --- → 3
Flags: qe-verify-
Flags: firefox-backlog+
Iteration: --- → 38.1 - 26 Jan
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
Attachment #8553367 - Flags: feedback?(dtownsend) → feedback+
Iteration: 38.1 - 26 Jan → 38.2 - 9 Feb
Depends on: 1126816
Posted patch Patch v2 (obsolete) — Splinter Review
Addressed comment 2. Now requesting review, as I'm fixing the Orange I mentioned in comment 1 in bug 1126816.
Attachment #8553367 - Attachment is obsolete: true
Attachment #8558018 - Flags: review?(dtownsend)
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
Attachment #8558018 - Flags: review?(dtownsend) → review+
(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.
Attachment #8558018 - Attachment is obsolete: true
Attachment #8559162 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/365129a004d7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 38
You need to log in before you can comment on or make changes to this bug.