search

mozilla / geckodriver

WebDriver for Firefox
https://firefox-source-docs.mozilla.org/testing/geckodriver/
Mozilla Public License 2.0
7.03k stars 1.51k forks source link

window variables aren't preserved in the injected script context (works under regular firefox) #2075

Closed karlicoss closed 1 year ago

karlicoss commented 1 year ago

System

Testcase

This repository contains a minimal extension + test.py script that helps to reproduce the bug

test.py does the follwing

Expected:

window.injected variable state is preserved on navigation back (thanks to bfcache )

the log in page console should be something like this

injecting: 1674087397881
already injected: 1674087397881

This is the case under chromedriver, and also under regular Firefox (if you load the extension manually) and do the steps above

Actual

window.injected variable state is lost on navigation back

the log looks like this:

injecting: 1674087216152 inject.js:4:12
injecting: 1674087220881 inject.js:4:12

Note that otherwise bfcache works as expected under geckodriver. E.g. if you modify the page in the devtools, navigate to other page and then press "back", the DOM will stay modified.

Misc

possibly relevant issue: https://github.com/mozilla/geckodriver/issues/1067 , but in my case I'm not injecting any code from geckodriver, so I feel like it's a separate issue

whimboo commented 1 year ago

I had a look and this is quite strange. First I thought that this might be related to a preference that geckodriver sets but even when I disable setting any preference before starting Firefox and any recommended preference in Marionette the behavior is the same.

One interesting observation that I did was that setting a breakpoint inside of inject.js which sets the injected boolean on the window object, allows me to run window.injected in the console and that prints the date as string after the property is set. But once the debugger leaves that file the same command in the console window returns undefined.

@juliandescottes maybe you have an idea if some kind of scope creep goes on here?

juliandescottes commented 1 year ago

Selenium starts marionette with the moz:debuggerAddress capability, meaning we should enable CDP. When this capability is set, geckodriver actually disables Fission at https://searchfox.org/mozilla-central/rev/7a9e3bbab8f81c2cbc72a394047f948da9cfef9a/testing/geckodriver/src/capabilities.rs#549-555

Setting fission.autostart = false on any Firefox installation will show the same issue as described in this ticket.

Several things from that observation

whimboo commented 1 year ago

Oh that's a very good finding! I completely missed that particular part. I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1813981 to get this fixed.

The plan here is to get rid of the extra code block which is no longer needed for Firefox since a couple of releases. Also we probably don't need any of the other two preferences as suggested by our CDP documentation given that Selenium only uses the Logging feature from CDP (whereby this is transitioning even to WebDriver BiDi now).

@karlicoss can you please modify your test and add the following lines? Does that work in your final testing environment?

    options = webdriver.FirefoxOptions()
    options.set_preference("fission.autostart", True)

    driver = webdriver.Firefox(firefox_binary=binary, options=options)
whimboo commented 1 year ago

This will be fixed in the upcoming geckodriver release.

karlicoss commented 1 year ago

I tried options.set_preference("fission.autostart", True), and it works both in the test I attached to the issue, and in my original code for promnesia. So I can remove the nasty hack where I used DOM to make it work with bfcache. Really appreciate it, it's such an obscure issue that I didn't really expect it to be fixed soon :black_heart:

whimboo commented 1 year ago

@karlicoss Please note that the preference is no longer necessary. We released an updated geckodriver by end of last week.