1903272 - Inappropriate "browsingContext" events for "moz-extension://" URLs

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just noticed when running the following command and manually playing with this W3Schools page:

mach run --setpref="remote.log.level=Trace" --remote-debugging-port

Here the event that I didn't expect to see when we are in content scope:

1718713281433	RemoteAgent	DEBUG	WebDriverBiDiConnection 8c1025ef-b9b6-461a-8c2e-1d3d21195d78 <- {"type":"event","method":"browsingContext.contextDestroyed","params":{"children":null,"url":"moz-extension://db5806b1-cada-4cad-9783-5191b570f14f/_generated_background_page.html","userContext":"default","parent":null}}

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Dan]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #0)

Just noticed when running the following command and manually playing with this W3Schools page:

mach run --setpref="remote.log.level=Trace" --remote-debugging-port

Here the event that I didn't expect to see when we are in content scope:

1718713281433	RemoteAgent	DEBUG	WebDriverBiDiConnection 8c1025ef-b9b6-461a-8c2e-1d3d21195d78 <- {"type":"event","method":"browsingContext.contextDestroyed","params":{"children":null,"url":"moz-extension://db5806b1-cada-4cad-9783-5191b570f14f/_generated_background_page.html","userContext":"default","parent":null}}

I don't seem to get the steps to reproduce this error. Can you please clarify? Thanks.

Comment 3

[Dan]

Okay, I could reproduce the error such as to get the event you mentioned. Can you recall the specific action you did on that page to cause this event. Here are the events that occured on that page when I interacted with it that relates to webdriver bidi

1723731296039   RemoteAgent     DEBUG   WebDriver BiDi enabled
1723731296039   RemoteAgent     TRACE   Received observer notification final-ui-startup
1723731296283   RemoteAgent     TRACE   Available local IP addresses: 127.0.0.1, [::1]
1723731296292   RemoteAgent     DEBUG   Setting recommended pref permissions.isolateBy.userContext to true
WebDriver BiDi listening on ws://127.0.0.1:9222

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Dan. So good to hear that you were able to get the client connected and running. To give a more detailed answer to your last comment I would need more content from the log. Especially around when a browsing context gets destroyed (frame removed, page navigated, or tab closed).

Regarding this bug we are not expecting an actual error to not be returned. It's the event that shouldn't be send to the client, and we have to filter out in the onContextDiscarded method.

The steps I did are roughly the following:

1. Connect the client to the other instance of Firefox
2. Run the session.new command with the default params
3. Run the session.subscribe command with the params: { events: ["browsingContext"] } (enables events for the browsingContext module)
4. Manually loading https://www.w3schools.com/jsref/tryit.asp?filename=tryjsref_onbeforeunload in the other Firefox
5. Playing around with the example and go back and forth in history

Sadly when running these steps I can no longer reproduce this issue. At the time when I saw it there was an extension with the id db5806b1-cada-4cad-9783-5191b570f14f installed. That is not the case anymore, and I cannot find out which extension that was, given that on mozilla-central we do not have an entry for it. Maybe it was some experiment which doesn't exist anymore.

I tried to install some other extensions with background pages but still I'm not able to reproduce the situation.

The best here might be to close the bug as WFM and we can reopen when it happens again. But then we should clearly check which extension is actually triggering it.

Dan, thanks for digging into that one. And we can find another bug for you to work on.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I was able to reproduce this event by running the following command via the DevTools console:

(async function() {
    await sendCommand("session.new", { capabilities: {} });

    await sendCommand("session.subscribe", {"events":["log.entryAdded","browsingContext.contextCreated","browsingContext.contextDestroyed"]});
    await sendCommand("script.addPreloadScript", {"functionDeclaration":"function customElementWrapper() {\n    const origFn = customElements.define.bind(customElements);\n    customElements.define = function (name, Constructor, options) {\n        class WdioWrapperElement extends Constructor {\n            connectedCallback() {\n                super.connectedCallback && super.connectedCallback();\n                let parentNode = this;\n                while (parentNode.parentNode) {\n                    parentNode = parentNode.parentNode;\n                }\n                console.debug('[WDIO]', 'newShadowRoot', this, parentNode);\n            }\n            disconnectedCallback() {\n                super.disconnectedCallback && super.disconnectedCallback();\n                console.debug('[WDIO]', 'removeShadowRoot', this);\n            }\n        }\n        return origFn(name, WdioWrapperElement, options);\n    };\n}"});
  
    let res = await sendCommand("browsingContext.getTree", {});

    const context = res.result.contexts[0].context;
    await sendCommand("browsingContext.navigate", {
      context,
      url: "https://guinea-pig.webdriver.io/",
      wait: "complete"
    });

    // Do stuff ...
  })()

It triggers a context destroyed event for moz-extension://703ee72b-83f4-411d-8eaa-f0a517d68377/_generated_background_page.html, but there is no extension installed at all. Also in the profile I cannot find any reference to this UUID. That is strange.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I was finally able to get more information about that particular extension. As it looks like it's not a regular one given that the uuid is not retrievable via the AddonManager. But using the moz-extension URL and opening it in the browser shows that it is the Report Site Issue extension:

https://searchfox.org/mozilla-central/source/browser/extensions/report-site-issue/

Maybe we should just exclude any browsing context that has extension set as currentRemoteType? Julian, do you have any objections?

Comment 7

[Julian Descottes [:jdescottes]]

That sounds fine, let's just make sure to add a comment pointing to Bug 1755014, as we will need to revisit all spots where we filter out webextension contexts when we want to officially support them in the future.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We already have the isExtensionContext helper, but it seems we aren't using it when sending out the browsing context events. I'm curious why we don't see the browsingContext.contextCreated event being sent when the extension's background page is created.

Comment 9

[Julian Descottes [:jdescottes]]

Moving to m14.

We should file a bug to see if it's possible to test extension pages by loading moz-extension URLs in a tab as well, as this seems to be a recommended solution to test extensions until we have support.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Julian Descottes [:jdescottes] from comment #9)

We should file a bug to see if it's possible to test extension pages by loading moz-extension URLs in a tab as well, as this seems to be a recommended solution to test extensions until we have support.

I filed bug 1959376 with the results of my initial investigation.

Comment 11

[Julian Descottes [:jdescottes]]

For the record I can no longer reproduce without installing additional extensions.

My STRs:

• start Firefox with WebDriver BiDi session
• subscribe to browsingContext events globally
• install uBlock Origin
• hover the extension's icon added in the toolbar

We get browsingContext.contextCreated, browsingContext.contextDestroyed and browsingContext.navigationFailed.
For the 2 last events (both originating from the browsing-context-discarded observer notification) we can easily skip the events by checking browsingContext.currentRemoteType === "extension" (although Nika mentions that this getter is a bit hacky at https://searchfox.org/mozilla-central/rev/4bd620c218a01568d541453ce3b215ebf55bca63/dom/chrome-webidl/BrowsingContext.webidl#299-303).
However for the first one, coming from the browsing-context-attached observer notification) the currentRemoteType is not set yet. Not window is attached to the browsing context either so we can't really assert anything.

However we could rely on the fact that we can't find any context id for this context, because it simply doesn't match any "tab" tracked by our Tab manager.

Nika, Luca, do you know if there is a good way to filter out BrowsingContexts created for webextensions? As said above currentRemoteType doesn't seem set yet in browsing-context-attached (could this be a bug?). Otherwise I would just rely on the fact that they don't match any tab tracked by WebDriver BiDi? This should be enough to avoid events coming from webextension popups / sidebars etc.

Comment 12

[Julian Descottes [:jdescottes]]

Expanding the scope to cover all browsingContext events.

Comment 13

[Nika Layzell [:nika] (ni? for response)]

What is the intention here? Is the idea to only respond to contexts which are for primary browser content?

I expect you might want to filter to check for browsing context events for BCs which have the "browsers" MessageManagerGroup, which indicates that they are actually in a primary content browser, rather than firing the event for any BC at all. Currently this is set a bit after the BC would be attached and fire the event, but in https://bugzilla.mozilla.org/show_bug.cgi?id=1965437#c6 I have some notes about what would need to change to make that happen earlier.

TBH though, I'm not sure you want arbitrary extensions to be listening to these events. When and how BrowsingContexts are created/destroyed is a bit of an internal implementation detail, and I don't think we want extensions to be able to observe e.g. browsing context swaps due to navigations, do we?

Comment 14

[Julian Descottes [:jdescottes]]

Attachment: Bug 1903272 - [bidi] Skip browsingContext events for webextension contexts

Comment 15

[Julian Descottes [:jdescottes]]

Attachment: Bug 1903272 - [wdspec] Add mozilla test checking no browsing context event emitted for webextension contexts

Comment 16

[Julian Descottes [:jdescottes]]

(In reply to Nika Layzell [:nika] (ni? for response) from comment #13)

What is the intention here? Is the idea to only respond to contexts which are for primary browser content?

I expect you might want to filter to check for browsing context events for BCs which have the "browsers" MessageManagerGroup, which indicates that they are actually in a primary content browser, rather than firing the event for any BC at all. Currently this is set a bit after the BC would be attached and fire the event, but in https://bugzilla.mozilla.org/show_bug.cgi?id=1965437#c6 I have some notes about what would need to change to make that happen earlier.

TBH though, I'm not sure you want arbitrary extensions to be listening to these events. When and how BrowsingContexts are created/destroyed is a bit of an internal implementation detail, and I don't think we want extensions to be able to observe e.g. browsing context swaps due to navigations, do we?

This is not exposed to webextensions.
This is about the WebDriver BiDi browsingContext.contextCreated and contextDestroyed events. Clients can listen to those events to be notified about new content contexts created (eg adding a new tab). We are not interested in BC swaps (we discard notifications emitted with why="replace" here).

My goal here is just to make sure I only send notification about browsing contexts which are relevant for WebDriver BiDi clients, so only content browsing contexts (tabs, frames etc...)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3d4eac99085
[bidi] Skip browsingContext events for webextension contexts r=webdriver-reviewers,Sasha
https://hg.mozilla.org/integration/autoland/rev/f26920a30bb2
[wdspec] Add mozilla test checking no browsing context event emitted for webextension contexts r=webdriver-reviewers,Sasha

Comment 18

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/f3d4eac99085
https://hg.mozilla.org/mozilla-central/rev/f26920a30bb2