Closed Bug 1578584 Opened 3 months ago Closed 2 months ago

WebExt API: Add onResultPicked event

Categories

(Firefox :: Address Bar, enhancement, P2)

enhancement
Points:
5

Tracking

()

RESOLVED FIXED
Firefox 71
Iteration:
71.2 - Sept 16 - 29
Tracking Status
firefox71 --- fixed

People

(Reporter: harry, Assigned: adw)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

This is an event in the browser.urlbar namespace that fires when a contextual tip button or help link is clicked on. It could possibly be passed a name/ID of the element/payload of the tip that was picked.

If clicks/keypresses on contextual tips are passed to the provider, then UrlbarProviderExtension can then forward them to the extension.

Priority: -- → P2
Assignee: nobody → adw
Status: NEW → ASSIGNED
Iteration: --- → 71.2 - Sept 16 - 29
Depends on: 1578439
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/abcfd108c7e5
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy

Just pushed a fix.

Flags: needinfo?(adw)
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/febf4480bc0b
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
Backout by aciure@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2f60380f9df2
Backed out changeset febf4480bc0b for causing browser_ext_urlbar.js to perma fail CLOSED TREE
Pushed by aciure@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0e4e5312bf5
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
Pushed by aciure@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4730635520c6
Follow-up test fix: Quantumbar WebExt API: Add onResultPicked event.
Attached image 1578584.png

The follow-up didn't fix the test failures. I don't understand. Here's a screenshot where I reproduced it locally before the follow-up fix. Instead of the single tip result being present, there are a couple of normal results. So I thought the problem was the extension notification timeout, which test_ext_urlbar.js increases. That's still the right thing to do, but I don't see why it didn't fix this.

I don't have time to look at this more before I'm afk for a week or two.

All this test does is load a mock extension that implements a tip provider, perform a query, and click a button in the tip. (It does this in each of its tasks.)

I debugged this more, and the problem is that sometimes the query happens before the provider is registered. That's why the two normal non-tip results appear. Why is the query happening before the provider is registered even though the test appears to be performing the query after registering the provider? As best I can tell, it's due to the process separation. The provider is registered once the parent process receives the addListener call from the extension. The test only waits for the addListener calls in the extension to return. That's not the same thing. (I tried awaiting each addListener call before sending the ready message to the test, but that wasn't enough.)

What I don't understand though is why test_ext_urlbar.js doesn't seem to be bit by this. It checks the provider immediately after awaiting extension startup: https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/browser/components/extensions/test/xpcshell/test_ext_urlbar.js#115

There are a couple of differences between that test and this one. It's an xpcshell test, and it calls addListener in the top-level background script scope. So maybe awaiting ext.startup() is just enough time for the parent process to receive the addListener call. Or maybe there's some relevant difference between mochitests and xpcshell tests when it comes to the extension test framework.

Here's a try push that just polls until the provider has been added: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e6d45e6f03647ce5c1dcbe976f6f378e144db476

Flags: needinfo?(adw)
Attachment #9095392 - Attachment is obsolete: true

I noticed that there were errors thrown in other tests that created tip providers due to the default pickResult implementation in UrlbarProvider, which throws an error. So I also added pickResult implementations to those providers. Here's a second try push with that fix too: https://treeherder.mozilla.org/#/jobs?repo=try&revision=14de03547ba9f2d4c30c965c97dcc43ed21d849b

There are failures in browser_tip_selection.js, browser_urlbar_resultSpan.js, and browser_urlbar_event_telemetry.js. I didn't think these were related to my patch, so I pushed again to try without my patch applied, but with the fix for bug 1578436 included: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c61534c673b816f438b44ad6b92fa2b288925b96

The same failures happened again, so they're not due to my patch, and it looks like we have some intermittent problems we need to fix.

Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5bdf5f9062a0
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
You need to log in before you can comment on or make changes to this bug.