1582597 - browser.urlbar API: Add a proper type for result payloads

Status: NEW

(Firefox :: Address Bar, enhancement, P3)

Description

[Drew Willcoxon :adw]

See the discussion in https://phabricator.services.mozilla.com/D46248#1408526. Right now, the result payload type is just "object". We should specify a separate payload type for each result type, or a single payload type with optional members to cover all the result types.

Comment 1

[Marco Bonardo [:mak] (Away 10-21 August)]

(In reply to Drew Willcoxon :adw from comment #0)

See the discussion in https://phabricator.services.mozilla.com/D46248#1408526. Right now, the result payload type is just "object". We should specify a separate payload type for each result type, or a single payload type with optional members to cover all the result types.

I disagree, it would be a pain to handle in the future when we add new types or modify the existing types, being a generic object is a feature to us.
The idea was that the translator (between the add-on and the urlbar code, that is likely UrlbarProviderExtension does validation and discards invalid results.

Comment 2

[Marco Bonardo [:mak] (Away 10-21 August)]

does the schema validation you added resolve some of the concerns here?

Comment 3

[Drew Willcoxon :adw]

Partly -- the urlbar.json schema in browser/components/extensions still defines the payload as simply object, so it's possible for extensions to pass in any type of object, but at least now we have schema validation in urlbar, so if an extension passes in an invalid payload, we'll throw. It looks like the exception will cause invalid results to be ignored and logged with Cu.reportError.

This bug is still valid I think because we could add more strongly typed payloads to urlbar.json. The question is, is that really worth our time? It probably isn't anymore? We've stopped adding new APIs to the browser/components/extensions implementation in favor of experimental APIs. And we don't have any immediate plans on opening up the urlbar API to third-party extensions.

What do you think? I would be fine closing this bug, but it might be a good idea to keep it around as a reminder in case we do open up the API to all extensions, or until we remove the browser/components/extensions implementation.