Create regression tests for preferred application selection in preferences/options

RESOLVED FIXED in Firefox 64



11 months ago
11 months ago


(Reporter: Gijs, Assigned: Gijs)


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

Firefox Tracking Flags

(firefox64 fixed)



(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
Priority: -- → P1
This try is green at time of writing, though macOS is still pending:
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
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:
> busted,
> exception&classifiedState=unclassified&revision=834bfb49df9a0950866ffcfc46460
> 4f2490ca0cb&selectedJob=201671271
> Failure log:
> html#?job_id=201671271&repo=autoland&lineNumber=1151
> Backout:
> 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 ``, 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:
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to :Gijs (out Thu 27 - Sun 30 / 9;  he/him) from comment #8)

try is greener than our lawn in spring, so relanding. :-)
Pushed by
remove use of feeds from application selection test, and check the dropdown actually works, r=paolo
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.