1632556 - When using geckodriver "navigator.webdriver" returns "false"

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P1)

Description

[jacktheripper1990]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.113 Safari/537.36

Steps to reproduce:

Opened a page using Selenium via Python binding

Actual results:

navigator.webdriver returns false

Expected results:

navigator.webdriver should true

Comment 1

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

As filed this lacks various information:

• Which version of Firefox, geckodriver and Selenium has been used?
• A minimized Selenium script
• A trace log

Comment 2

[jacktheripper1990]

Attachment: tracelog

Run on MacOS Catalina 10.15.4

- Firefox: 75.0 (64-bit)
- geckodriver 0.26.0
- Selenium: pip package, version 3.141.0
- Script:

```python
from selenium import webdriver

profile = webdriver.FirefoxProfile()
profile.set_preference("devtools.console.stdout.content", True)
profile.update_preferences()

driver = webdriver.Firefox(firefox_profile=profile)

driver.get("https://example.com")
driver.execute_script("console.warn(\"Webdriver value: \" + navigator.webdriver)")
```

Trace log
```


in geckodriver.log: `console.log: "Webdriver value: false"`

Comment 3

[jacktheripper1990]

Run on MacOS Catalina 10.15.4

• Firefox: 75.0 (64-bit)
• geckodriver 0.26.0
• Selenium: pip package, version 3.141.0
• Script:

from selenium import webdriver

profile = webdriver.FirefoxProfile()
profile.set_preference("devtools.console.stdout.content", True)
profile.update_preferences()

driver = webdriver.Firefox(firefox_profile=profile)

driver.get("https://example.com")
driver.execute_script("console.warn(\"Webdriver value: \" + navigator.webdriver)")

in geckodriver.log: console.log: "Webdriver value: false"

Trace log attached.

Comment 4

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

Thanks. I can actually confirm that behavior with geckodriver, but using Marionette directly seems to work:

1587668131698	Marionette	DEBUG	3 -> [0,3,"WebDriver:ExecuteScript",{"script":"return navigator.webdriver","newSandbox":true,"args":[],"filename":"_a/test_minimized.py","sandbox":"default","line":51}]
1587668131707	Marionette	DEBUG	3 <- [1,3,null,{"value":true}]

With geckodriver the invocation of the command looks like:

1587667905065	Marionette	DEBUG	0 -> [0,3,"WebDriver:ExecuteScript",{"args":[],"script":"return navigator.webdriver"}]
1587667905082	Marionette	DEBUG	0 <- [1,3,null,{"value":false}]

Using new_sandbox or sandbox as arguments doesn't matter. There is something else which prevents it from returning true.

Comment 5

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

Actually it regressed by bug 1622012 for Firefox 75. Lets get if we can get this fix into 76, and create a webdriver test for it.

Comment 6

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

Specifically the change in https://hg.mozilla.org/mozilla-central/rev/0656956fe08b caused it. I'll check that tomorrow.

Comment 7

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

Workaround for now is to set the marionette.enabled preference to True. I still wonder why the marionette client itself is not affected by that.

Comment 8

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

Now that we read the pref only once and then determine the enabled state of Marionette based on the environment variable or command line argument, we do not write the marionette.enabled pref anymore. As such the following code fails:

https://searchfox.org/mozilla-central/rev/41c3ea3ee8eab9ce7b82932257cb80b703cbba67/dom/base/Navigator.cpp#2097-2099

I would suggest that we get an instance of the marionette component and retrieve the actual enabled state.

Comment 9

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

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

I would suggest that we get an instance of the marionette component and retrieve the actual enabled state.

For a short-term fix lets get the preference updated, and fix it properly by removing it completely as stated above. I filed bug 1632821 as follow-up.

Comment 10

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

Attachment: Bug 1632556 - [marionette] "navigator.webdriver" has to reflect the enabled state of Marionette.

Comment 11

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

Attachment: Bug 1632556 - [wdspec] Added webdriver test for navigator.webdriver being enabled.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/14f317d6d48d
[marionette] "navigator.webdriver" has to reflect the enabled state of Marionette. r=marionette-reviewers,maja_zf
https://hg.mozilla.org/integration/autoland/rev/2e8129581f3f
[wdspec] Added webdriver test for navigator.webdriver being enabled. r=maja_zf

Comment 13

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/23236 for changes under testing/web-platform/tests

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 15

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 16

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/14f317d6d48d
https://hg.mozilla.org/mozilla-central/rev/2e8129581f3f

Comment 18

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

Comment on attachment 9143085 [details]
Bug 1632556 - [marionette] "navigator.webdriver" has to reflect the enabled state of Marionette.

Beta/Release Uplift Approval Request

• User impact if declined: User agents under control by WebDriver have to inform the document that it is under automation. This broke with bug 1622012 and always returns false since Firefox 75.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The check depends on the marionette.enabled preference, which we forgot to set by the changes on bug 1622012. This patch simply re-adds that setting. A patch to get rid of the preference will be followed upon by bug 1632821.
• String changes made/needed: none

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9143085 [details]
Bug 1632556 - [marionette] "navigator.webdriver" has to reflect the enabled state of Marionette.

Re-adds a preference check which was erroneously removed. Approved for 76.0rc1.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e9bf32038049
https://hg.mozilla.org/releases/mozilla-beta/rev/a7e9d43b44eb