1171256 - Add an API similar to Chrome's webNavigation

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

This needs some tests, but I'd like to get some initial feedback.

Comment 1

[Dave Townsend [:mossop]]

Comment on attachment 8614970 [details] [diff] [review]
patch

Review of attachment 8614970 [details] [diff] [review]:
-----------------------------------------------------------------

Looking good so far

::: toolkit/modules/addons/WebNavigation.jsm
@@ +24,5 @@
> +  init() {
> +    Services.mm.addMessageListener("Extension:DOMContentLoaded", this);
> +    Services.mm.addMessageListener("Extension:StateChange", this);
> +    Services.mm.addMessageListener("Extension:LocationChange", this);
> +    Services.mm.loadFrameScript("resource://gre/modules/WebNavigationContent.js", true);

If you do this and a tab already has that framescript loaded does it get loaded a second time?

@@ +49,5 @@
> +
> +  removeListener(type, listener) {
> +    let listeners = this.listeners.get(type);
> +    listeners.delete(listener);
> +    if (listeners.size == 0) {

This will throw if an extension double-removes. Maybe that's ok though it's probably a bad exception for the developer to decipher.

@@ +181,5 @@
> +  },
> +  removeListener(listener) {
> +    return Manager.removeListener("onReferenceFragmentUpdated", listener);
> +  }
> +};

A lot of duplication here that you could probably simplify:

for (let event of ["onBeforeNavigate", ...]) {
  WebNavigation[event] = {
    addListener: Manager.addListener.bind(Manager, event),
    removeListener: Manager.removeListener.bind(Manager, event),
  }
}

@@ +205,5 @@
> +
> +  // Not sure how to implement these yet.
> +  //onCreatedNavigationTarget,
> +  //onHistoryStateUpdated,
> +};

Also note getFrame and getAllFrames

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch v2

OK, here's a new version with a test.

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8617685 [details] [diff] [review]
patch v2

Review of attachment 8617685 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. The chrome docs talk about some filtering options for all this but I can't find any examples of it in the docs or examples. Do you know anything about it?

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7eca38a74be8

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7eca38a74be8

Comment 6

[Will Bamberg [:wbamberg]]

-> https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webNavigation