I was informed by the Puppeteer team today that the following code which is used to set a page's content doesn't trigger any navigation events via WebDriver BiDi. It's used a lot in Puppeteer and actually works when running with Chrome.

    await this.evaluate(html => {
      document.open();
      document.write(html);
      document.close();
    }, html);

Even through this API is not pretty and removes all the registered event listeners it is also used in web pages and as such we need to support it.

Given that we have our listeners attached from within a script running in chrome context, it should basically continue to work. If not we may have to register them via the windowRoot.

I'll create some test cases including documentElement.remove() and using the DOMParser to add new content to see how those scenarios work.

We may have to add this to our M7 milestone. Lets discuss on Monday.

FWIW, I quickly tested two things:

• if I subscribe to the event after document.open() is used, I get a load event when document.close() is called, so platform fires an event
• using this.#window = win.windowRoot.ownerGlobal; for the LoadListener window does not help with this issue

Looking at CDP, it seems that we use ChromeEventHandler to listen to load events, but I didn't manage to make it work for BiDi. I'm assuming our current CDP implementation works for the pattern from Puppeteer, so it might still be worth investigating.

Thanks Julian! Looks like that we might indeed can pick what we have in our CDP implementation. Beside the load event we would also need the navigationStarted and domContentLoaded. I hope that this would be easy enough as well - or maybe you only tested for load?

Here an example WebDriver BiDi test:

async def test_document_write(bidi_session, subscribe_events, top_context, wait_for_event):
    await subscribe_events(events=[DOM_CONTENT_LOADED_EVENT])

    on_entry = wait_for_event(DOM_CONTENT_LOADED_EVENT)

    await bidi_session.script.evaluate(
        expression="""document.open(); document.write("<h1>Replaced</h1>"); document.close();""",
        target=ContextTarget(top_context["context"]),
        await_promise=False
    )

    event = await on_entry

Another case could be the following:

    await bidi_session.script.call_function(
        function_declaration="""(html) => {
                const parser = new DOMParser();
                const doc = parser.parseFromString(html, "text/html");
                const errorNode = doc.querySelector("parsererror");
                if (errorNode) {
                    throw new Error("Failed");
                } else {
                    document.documentElement.replaceWith(doc.documentElement);
                }
            }
            """,
        arguments=[{"type": "string", "value": "<h1>Replaced</h1>"}],
        target=ContextTarget(top_context["context"]),
        await_promise=False
    )

Here we also don't fire events, but the question is if we should do or not. I haven't checked if platform is actually doing that.

Managed to make this work with BiDi as well. We can grab the chromeEventHandler via window.docShell.chromeEventHandler, but then it's important to use capture: true for those events.

For info, I tried the second test case with documentElement.replaceWith and in this case we don't fire a load event, even with the changes which allow the document.open/close approach to work.

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

For info, I tried the second test case with documentElement.replaceWith and in this case we don't fire a load event, even with the changes which allow the document.open/close approach to work.

Yes, that works differently and as told by Olli it's expected that no load event is fired. As such we should keep the document.open() approach on our focus for now given that this is a highly important feature for Puppeteer.

Event listeners are removed when using document.open, so we should use chromeEventHandler instead

Depends on D173020

Comment on attachment 9324157 [details]
Bug 1822772 - [messagehandler] Destroy MessageHandlers when pages move to BFCache

Revision D173132 was moved to bug 1823670. Setting attachment 9324157 [details] to obsolete.

Depends on D173020

The new tests were failing on Android because the previous (expected fail) test was leaving the test harness in a bad state.

https://hg.mozilla.org/mozilla-central/rev/56721ff2d9f3
https://hg.mozilla.org/mozilla-central/rev/eaa0d06fff8d
https://hg.mozilla.org/mozilla-central/rev/379e1d5e3b79