1361327 - Omnibox API which would allow overriding the Autocomplete Popup

Status: NEW

(WebExtensions :: General, enhancement, P5)

Description

[lucian]

Attachment: omnibox.dropdown_override.diff

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170330140409

Steps to reproduce:

Overriding the Autocomplete Popup in pre-WebExtension world was a quite powerful feature and it would be great to offer a similar solution as a WebExtension API

Comment 1

[lucian]

Hi guys,

First of all the attached patch should be considered as a proof of concept and not something which is polished by any means.

This work is the first proposal of a autocomplete popup using a new WebExtensions API. Besides the attached patch there is also a demo WebExtension using this new API - https://www.dropbox.com/s/xcchy54rvh9lk20/omnibox%40cliqz.com.xpi?dl=0

The patch was heavily inspired by Drew’s code (thanks for the awesome work! ) 
 - https://docs.google.com/document/u/1/d/15W-aPNRkJrl_zx_f_Ugg1QGV4l2lapflOwHxJa-U9u4/pub
 - https://hg.mozilla.org/users/dwillcoxon_mozilla.com/urlbar-api-toy-addon/
 - https://bugzilla.mozilla.org/show_bug.cgi?id=1257550

I wouldn’t go into too many details for the patch yet - probably a better start is discussing the proposed API so jumping directly there (please check the XPI file above):
- Registration of the popup is done directly in manifest.json 
  "omnibox": {
    "keyword" : "q",
    "dropdown_override": "sample/index.html"  
  },
- The registration is done first come first serve if more extensions want to take over the dropdown

- “urlbar” is exposed to the popup with the following methods
  1.  urlbar.getPanelHeight() -> integer
  2.  urlbar.setPanelHeight(integer val)
  3.  urlbar.getValue() -> string
  4.  urlbar.setValue(string val)
  5.  urlbar.getMaxResults() -> integer
  6.  urlbar.setMaxResults(integer val)
  7.  urlbar.getSelectionStart() -> integer
  8.  urlbar.setSelectionStart(integer val)
  9.  urlbar.getSelectionEnd() -> integer
  10. urlbar.setSelectionEnd(integer val)
  11. urlbar.setSelectionRange(start, end)
  12. urlbar.focus()
  12. urlbar.blur()

- Events emitted into the popup  
  1. reset           - sent for each new results set / query once
  2. results         - one event for each result item in a result set
                         - data: { index, url, action, image, title, type, text }
  3. urlbar_keydown  - keydown event in the urlbar
  4. urlbar_input    - input event for the urlbar
  5. urlbar_focus    - focus event for the urlbar
  6. urlbar_blur     - blur event for the urlbar

Open TODOs:
1. We need to find a way to access the urlbar even from outside this popup (example use case: get the current value or focus it from the background page)
2. Expose the other web extensions APIs (browser. ) in the popup and make the popup accessible though chrome.extension.getViews(). (It was working before so this might be a regression on central or I miss something)
3. Find a way to send the search status with the results (ongoing, complete, …)
4. Fully implement a complex case which would prove that everything needed is there (eg: CLIQZ Test Pilot experiment)
5. Find more missing events or needed hooks and implement them
6. Make the patch more decent and get it ready for a real review 
7. Discussion: “keyword” is currently required in order to have access to the omnibox APIs - https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/omnibox
(addons.webextension.<unknown> ERROR Loading extension 'null': Reading manifest: Error processing omnibox: Property "keyword" is required ). Would it be fine if we make either keyboard or dropdown required?

Looking forward to getting some feedback!

Cheers,
Lucian

Comment 2

[Caitlin Neiman [:caitmuenster]]

Hi Lucian, this has been added to the agenda for the June 13 WebExtension APIs triage meeting. Would you be able to join us? 

Wiki: https://wiki.mozilla.org/Add-ons/Contribute/Triage#Next_Meeting

Agenda: https://docs.google.com/document/d/1A_M0YD86Plcs6eHyM2KXkDXY074BHZ3fZvaWXCljQLI/edit#

Comment 3

[lucian]

Thanks for the heads up Caitlin! 
Unfortunately I will not be able to attend Tuesday at that time. As I guess we cannot reschedule that meeting please let me know if I can provide more input besides the details in this ticket.

Comment 4

[Tim Nguyen :ntim]

I think a more maintainable API would be allowing to use an iframe as autocomplete popup.

Comment 5

[Tim Nguyen :ntim]

(In reply to Tim Nguyen :ntim from comment #4)
> I think a more maintainable API would be allowing to use an iframe as
> autocomplete popup.

actually, this is how the proposed api works already.

Comment 6

[lucian]

Hi Tim, Thanks for checking this!
You are right - this proposal uses an iframe to display the result exactly for the reason you mentioned - easier to maintain both on Firefox side and on consumer side. 
Looking forward to getting more feedback :)

Comment 7

[Andy McKay]

I'd like to bounce this over to Panos or someone in his team, providing an API like this is more a question of what the maintainers of the awesome bar would like to support. We've worked quite a bit with Marco on that in the past. 

We have some concerns about the overall architecture, making sure its responsive, security issues and ensuring that it doesn't escape the WebExtensions sandbox. But APIs like this can't be built in isolation and not without the input of other people who work on and maintain this area.

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Drew, Marco, is the proposed web extension API something we can commit to support?

Comment 9

[Drew Willcoxon :adw]

The API looks very similar to the one I proposed, as Lucian says, so IMO it's one we can support. :-)  It has a fairly small surface in terms of methods and events, and they're all pretty primitive (as opposed to complex), so I don't think it would be too hard to maintain.  And the TODOs in comment 1 sound reasonable.

I know that Marco disagreed though and thought that the API should not allow for so much flexibility -- I hope I'm not mischaracterizing what he said earlier.  At least he didn't like the idea of extensions having to recreate "stock" results, which would seem to imply an API that slots in custom results among stock results instead of one that replaces the popup with an iframe.

Comment 10

[Marco Bonardo [:mak]]

my thought was not much about disallowing flexibility, but about the fact by just exposing an iframe, we basically require each consumer to recreate the awesomebar code and:
1. provide interaction and accessibility
2. provide a firefox compatible design and coherent experience

And since all the consumers so far are using a row-view very close to what we have (just with more contents per row, maybe with a side information panel or multi selection in each row), long term it would be much cleaner and simpler for consumers if content providers could just provide contents and we would take care of their presentation.
The API I was looking for was mostly asking content providers to provide an html fragment for each result (even a complex one, with a side info panel, horizontal-multi-selection and so on), and the Firefox location bar to take care of all the presentation and accessibility concerns. We'd provide the possibility to add free form results to the awesomebar, in practice.

On the other side, my approach requires far more coding and time on our side, so it's quite unlikely to be feasible for 57.

Btw, going back on track about feedback, it's unclear to me whether this API will have access to the Panel.urlbar object (sounds like it does through the _panel.urlbar property?).
If so this is basically giving away the gURLBar object. Is that compatible with the kind of abstraction required by WebExtensions?
Sounds like this would break at every important gURLBar API change. And I'm not even sure it's going to work from another process.

Can that be clarified please?

Comment 11

[Tim Nguyen :ntim]

Marco, I think flexibility is nice to have, it would be nice to be able to do a safari style popup for the URLbar for example.

Comment 12

[Marco Bonardo [:mak]]

(In reply to Tim Nguyen :ntim from comment #11)
> Marco, I think flexibility is nice to have, it would be nice to be able to
> do a safari style popup for the URLbar for example.

Nothing that couldn't be done by providing one rich html fragment per result (A result could be just a disabled label for example). Indeed it would even be simpler to use imo. The only thing you couldn't do is a freeform-grid-like presentation (would that be even useful?).
Btw, as I said I don't think we have the resources to implement a clean API for that in a meaningful time, so I'm fine with leaving the burden of presentation to the add-ons for now.

I'm still doubtful this API will cover the needs and requirements if it need to expose the gURLBar object though, so I'll wait for an answer to that.

Comment 13

[Marco Bonardo [:mak]]

Also, the html fragment per result solution is what I think we should try to have in Firefox itself, regardless of add-ons. It's clear sooner or later we'll want to integrate richer results ourselves.

Comment 14

[Drew Willcoxon :adw]

(In reply to Marco Bonardo [::mak] from comment #10)
> Btw, going back on track about feedback, it's unclear to me whether this API
> will have access to the Panel.urlbar object (sounds like it does through the
> _panel.urlbar property?).

That's a good point I want to comment on too: the API should absolutely not allow access to the underlying Firefox implementation of the urlbar.  There's little point in having a separate extensions API otherwise.

Comment 15

[lucian]

(In reply to Marco Bonardo [::mak] from comment #10)
> Btw, going back on track about feedback, it's unclear to me whether this API
> will have access to the Panel.urlbar object (sounds like it does through the
> _panel.urlbar property?).
> If so this is basically giving away the gURLBar object. Is that compatible
> with the kind of abstraction required by WebExtensions?
> Sounds like this would break at every important gURLBar API change. And I'm
> not even sure it's going to work from another process.
> 
> Can that be clarified please?

This WebExtension API should definitely not expose the gURLBar to the web extension scope. The only exposed area is a limited set of methods needed for the dropdown (iframe) to function. These methods are described in comment #1 . The 'urlbar' in there is not the actual urlbar/gURLBar but rather an abstraction of it.

Regarding the row-view suggestion from comment #10 - agree with Marco that the complexity of implementing something like that is much higher while, in my opinion, would have a big hit on flexibility. A very important complexity of that suggestion is the ranking which would be quite hard to handle. 

As it seems that this proposal can actually be considered for Firefox 57 I would suggest to move it forward and try to solve some of those TODO in comment #1 . 

Marco, Drew: do you agree?

Comment 16

[Marco Bonardo [:mak]]

(In reply to lucian from comment #15)
> This WebExtension API should definitely not expose the gURLBar to the web
> extension scope.

That's good, thank you for the clarification.

> Regarding the row-view suggestion from comment #10 - agree with Marco that
> the complexity of implementing something like that is much higher while, in
> my opinion, would have a big hit on flexibility. A very important complexity
> of that suggestion is the ranking which would be quite hard to handle. 

I sort-of disagree with the flexibility statement, but I totally agree with the fact going my alternative plan is too expensive.
As I previously said, I'm fine with the short term iframe solution, let's see how it works in the wild.

Comment 17

[Drew Willcoxon :adw]

That sounds good to me, Lucian.

Comment 18

[lucian]

Thanks guys! Will come back soon with a more decent solution following the protocol described above.

Comment 19

[vz]

Attachment: omnibox.dropdown_override.2.diff

Hi guys!

So here is the more decent solution Lucian promised :) To see it in action use this sample extension: https://www.dropbox.com/s/n2mu6yozjkc31f9/omnibox%40cliqz.com.2.xpi?dl=0

Basically I took Lucian's implementation as a base and tried to turn it into a more WebExtensions-like solution. I kept the same way of taking over the omnibox through overriding popup's "_invalidate" method, but Instead of injecting API object into iframe document I made omnibox page work under the hood the same way as all the other webextension popups or "new tab" pages do. It allows using extension API inside it. This page can also be accessed from the other extension pages via the regular extension.getViews().

Also we thought that it would be better to change the API to make it look more like "WebExtensions". Now it looks like this:

METHODS:
--------
All following methods except getMaxResults and setMaxResults can take optional first argument windowId to specify a window which omnibox should be updated. If it is omitted we assume it to be the same window as where this method is being called from.

When calling this method from background page window ID should always be specified.

  omnibox.setMaxResults(integer maxResults)
    Update max results number, globaly for all windows

  omnibox.getMaxResults() -> Promise<integer>
    Get current max number of search results

  omnibox.get([integer windowId]) -> Promise<OverrideDetails>
    Get parameters of the omnibox override page. Replaces all "urlbar.get*" 
    methods from previous implementation.

  omnibox.update([integer windowId], OverrideDetails details)
    Update parameters of the omnibox override page for specified window ID. 
    Replaces all urlbar.set* methods from previous implementation
  
  omnibox.focus([integer windowId])
    Set focus to urlbar belonging to window with specified ID.

  omnibox.blur([integer windowId])
    Set focus to urlbar belonging to window with specified ID.

  omnibox.enter([integer windowId])
    Perform a default browser action to query in urlbar 
    of the window with specified ID.

EVENTS:
-------
All extension pages are able to listen for the omnibox events only from the same window they work in. Background page captures events from all omniboxes.

  omnibox.onInput -> { windowId, value }
    Fires when urlbar value changes. Event data contains new value.
  
  omnibox.onKeydown -> {KeyboardEventDetails}
    Fires when key is pressed in urlbar.

  omnibox.onFocus -> {windowId}
    Fires when urlbar gets focus.
  
  omnibox.onBlur -> {windowId}
    Fires when urlbar loses focus.

  omnibox.onReset -> {windowId}
    Fires when new search is initiated. 

  omnibox.onResults -> {Results}
    Fires when a new portion of search results is found for the current query.
    Every event object contains ALL results currently available for this query 
    (including results that were sent earlier).

TYPES:
------
  omnibox.OverrideDetails
    {number} windowId         ID of the window this omnibox belongs to.
    {string} value            Current urlbar value
    {integer} height          Dropdown height
    {integer} selectionStart  Urlbar selection start position
    {integer} selectionEnd    Urlbar selection end position

  omnibox.KeyboardEventDetails
    {number} windowId         ID of the window this omnibox belongs to.
    {string} key              Key pressed
    {string} code             Key code
    {boolean} altKey          True if Alt key is pressed
    {boolean} ctrlKey         True if Ctrl key is pressed
    {boolean} metaKey         True if Meta key is pressed
    {boolean} shiftKey        True if Shift key is pressed

  omnibox.Result
    {string} url              URL of result
    {string} image            Search result icon url
    {string} title            Search result title
    {string} type             Search result type
    {object|null} action      Parsed moz-action (if exist)

  omnibox.Results
    {number} windowId         ID of the window this omnibox belongs to.
    {string} query            Search query
    {integer} searchStatus    Search status
    {Array<Result>} results   Array of results

  omnibox.SearchStatus {integer}



From our perspective this solution looks much better. What do you guys think?

Vladimir

Comment 20

[lucian]

Hi guys! Can you please give us some feedback on our latest proposal? A view on both the omnibox functionality changes and on the WebExtension API changes would be very helpful and would help us move this forward.
Thanks!

Comment 21

[Tim Nguyen :ntim]

I preferred the previous proposal for its flexibility.

Comment 22

[Tim Nguyen :ntim]

Forget my previous comment, I changed my mind after looking at the patch :)

Comment 23

[Drew Willcoxon :adw]

Looks OK to me.  Making it more WebExtensions-like is the right idea I think.

Comment 24

[Marco Bonardo [:mak]]

I just had a quick look, didn't have time to go deep into details, so far the proposal in comment 19 looks good to me.

Comment 25

[lucian]

Thanks guys! This is awesome :)
What should be the next steps? Could we get some feedback from the addons team?

Comment 26

[Kev Needham [:kev]]

Sorry Lucian, for a lack of feedback on the work. Technically it looks good, but we need to evaluate impacts of releasing as an API, and any maintenance requirements over time. I'm adding Mike Conca to this bug, and will talk to him about what's required and when. P5 is appropriate pending review.

Comment 27

[lucian]

Thanks Kev! Will be standing by ready to support with whatever is required.

Comment 28

[Mike Conca [:mconca]]

There is no desire to land this experiment as a public API at this time. However, this seems like a perfectly reasonable approach in the short-term to move bootstrapped extensions to a WebExtension-based implementation. Other folks will be reaching out with more details.

Comment 29

[Brett Zamir]

Btw, would you be open for now to a much smaller subset of this which would basically allow an omnibox to behave in the same way as with an omnibox extension but which avoided the need for a keyword (or better yet, provided a user-facing option by which they could indicate a keyword that would override the default)?

My extension https://github.com/brettz9/enumerate shows Google results within the URL bar and it would be nice for the user to avoid having to type "g" first to activate it given that such broad-use omnibox extensions might be preferred as the default...

Comment 30

[Firefox Bug Husbandry Bot]

Bulk move of bugs per https://bugzilla.mozilla.org/show_bug.cgi?id=1483958

Comment 32

[08xjcec48]

:ntim , you've closed #1262843 which was about only the layout of the dropdown items, while this bug is about adding custom stuff the dropdown, like search results. Are you sure this is a duplicate and merely changing the order of entries (URL - title instead of title - URL) will be covered by this?