Closed Bug 1578584 Opened 1 year ago Closed 1 year ago

WebExt API: Add onResultPicked event


(Firefox :: Address Bar, enhancement, P2)




Firefox 71
71.2 - Sept 16 - 29
Tracking Status
firefox71 --- fixed


(Reporter: harry, Assigned: adw)


(Blocks 1 open bug)



(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
Iteration: --- → 71.2 - Sept 16 - 29
Depends on: 1578439
Pushed by
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy

Just pushed a fix.

Flags: needinfo?(adw)
Pushed by
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
Backout by
Backed out changeset febf4480bc0b for causing browser_ext_urlbar.js to perma fail CLOSED TREE
Pushed by
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
Pushed by
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:

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:

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:

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:

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
Quantumbar WebExt API: Add onResultPicked event. r=harry,mixedpuppy
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
You need to log in before you can comment on or make changes to this bug.