Create regression tests for preferred application selection in preferences/options

RESOLVED FIXED in Firefox 64

Status

()

enhancement
P1
normal
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

Trunk
Firefox 64
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(1 attachment)

The applications settings are undermaintained - they've kind of always been there and we've fixed things in them when we've broken them, but I would go so far as to say that very few people understand all of the code that governs that list, the editable items, and how that all works.

We broke this when doing some routine refactoring of enumerators because the code is highly magical. We didn't notice because there is no (or at least not sufficient) test coverage. That should change.
Flags: in-testsuite+
I'll try biting this bullet and down-prio'ing if it proves too messy.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Priority: -- → P1
This try is green at time of writing, though macOS is still pending:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=675195fe799d41e0cb72aab077dfff947b0adaa3
I did run into some issues with `alwaysAsk` not being set on the handler info returned by the external protocol service (or the handler service). I can reproduce that when testing manually on my "normal" beta profile, though even with the value being "wrong" (set to 2, ie useDefault), the alwaysAsk behavior does happen when I try to open mailto links.
Comment on attachment 9011132 [details]
Bug 1491436 - remove use of feeds from application selection test, and check the dropdown actually works, r?paolo

:Paolo Amadini has approved the revision.
Attachment #9011132 - Flags: review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/834bfb49df9a
remove use of feeds from application selection test, and check the dropdown actually works, r=paolo
(In reply to Noemi Erli[:noemi_erli] from comment #7)
> Backed out changeset 834bfb49df9a (Bug 1491436) for failing in
> browser_applications_selection.js CLOSED TREE
> 
> Push with failures:
> https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed,
> busted,
> exception&classifiedState=unclassified&revision=834bfb49df9a0950866ffcfc46460
> 4f2490ca0cb&selectedJob=201671271
> 
> Failure log:
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=201671271&repo=autoland&lineNumber=1151
> 
> Backout:
> https://hg.mozilla.org/integration/autoland/rev/
> f41fce9645d1360b46f38c9109b48f55f47828b0

Turns out there's a subtle bug in the test:

  // select one of our test cases:
  list.selectedItem = list.querySelector("menuitem[label='Handler 1']");
  let {preferredAction, alwaysAskBeforeHandling} = HandlerServiceTestUtils.getHandlerInfo(itemType);
  Assert.notEqual(preferredAction, Ci.nsIHandlerInfo.alwaysAsk,
      "Should have selected something other than 'always ask' (" + itemType + ")");

The querySelector call returns null. This makes the selection a no-op, or at least, the result depends on the OS / previous option selected. Unfortunately, all my trypushes excluded win7 (win32) and therefore missed that on that OS only, mailto defaults are (apparently) different, and as a result, the test behaves differently. I would have noticed if I'd used the new helper I made in the end, because it uses `element.click()`, so passing it a null element breaks the test.

I changed the test as follows:

-  list.selectedItem = list.querySelector("menuitem[label='Handler 1']");
+  let handlerItem = list.querySelector("menuitem[label*='Handler 1']");
+  await selectItemInPopup(handlerItem);

and that seems to work locally. I pushed to try again: 

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94078f2276ea84691d5730ea232448dd15224ea8
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs (out Thu 27 - Sun 30 / 9;  he/him) from comment #8)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=94078f2276ea84691d5730ea232448dd15224ea8

try is greener than our lawn in spring, so relanding. :-)
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/23841605d322
remove use of feeds from application selection test, and check the dropdown actually works, r=paolo
https://hg.mozilla.org/mozilla-central/rev/23841605d322
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
You need to log in before you can comment on or make changes to this bug.