SeleniumHQ / selenium

A browser automation framework and ecosystem.
https://selenium.dev
Apache License 2.0
29.73k stars 8.02k forks source link

Selenium JavaScript atoms, used in appium-remote-debugger, are clobbering window._ causing issues with hybrid apps after webview context is utilized #14190

Closed ahalbrock closed 4 days ago

ahalbrock commented 4 days ago

What happened?

The javascript atoms here are used, I'm told, verbatim in the appium-remote-debugger project. When using Appium 2+ and switching the xcuitest driver for automation over to a webview context, these atoms are used to interact with the page. In the past "this" was given a cherry picked version of window as "this", one that really just contained window.navigator and window.document, but the newest versions seem to just pass "window" as is for "this".

The atoms seem to follow a pattern of having a main driving function defined that wrangles together all the other code present in the atom assigned to what is basically "this." and then end with a "return this..apply(null, arguments)" The previous cherry picked version of window (a new object that referenced window.navigator and window.document) didn't run into issues since the cherry picked object assigned to "this" didn't have "" defined, but for any application that does have window. defined, this causes underscore to be overwritten and can cause all kinds of issues.

If instead the driving function is not assigned to "this._" but instead invoked directly, e.g. "return DRIVINGFUNC.apply(null, arguments)", this clobbering of window. does not occur and we don't see any consequences.

How can we reproduce the issue?

Unfortunately I can't really share much specific code around this, but we have a hybrid iOS app that is being automated through Katalon 9+.  I've ruled out Katalon as a culprit here by performing the same test in a java only project and getting the same results.  After switching to the webview context seemingly any atom run, e.g. driver.click or driver.find_elements or pretty much any atom I've seen, will cause, in a round about way, this._ to be overwritten with the main function of the atom.

In our scenario, we fill out a couple pieces of text information on an initial page and click next.  The next triggers some further asynchronous code to run which includes calls to underscore utility methods like "_.isArray(...)" and will produce error message complaining that "_.isArray is not a function. (In '.isArray(fromArray)', '_.isArray' is undefined)"

If I either replace the current atom with one used previously that passes "this" as the cherry picked version of "window", or I modify the atom to not assign the main atom function to this._ and instead just call it directly, we do not receive these errors around "_".

Relevant log output

[9e0d3632][ios-device] Received message from Web Inspector:
[9e0d3632][ios-device] {
  "__argument": {
    "WIRApplicationIdentifierKey": "PID:1885",
    "WIRMessageDataTypeKey": "WIRMessageDataTypeFull",
    "WIRDestinationKey": "FOO",
    "WIRMessageDataKey": "{\"method\":\"Target.dispatchMessageFromTarget\",\"params\":{\"targetId\":\"page-14\",\"message\":\"{\\\"method\\\":\\\"Console.messageAdded\\\",\\\"params\\\":{\\\"message\\\":{\\\"source\\\":\\\"console-api\\\",\\\"level\\\":\\\"error\\\",\\\"text\\\":\\\"\\\",\\\"type\\\":\\\"log\\\",\\\"line\\\":1540,\\\"column\\\":28,\\\"url\\\":\\\"file:///private/var/containers/Bundle/Application/FOO/Application.app/www/cordova.js\\\",\\\"repeatCount\\\":1,\\\"timestamp\\\":1719326256.356053,\\\"parameters\\\":[{\\\"type\\\":\\\"string\\\",\\\"value\\\":\\\"\\\"},{\\\"type\\\":\\\"string\\\",\\\"value\\\":\\\"ERROR\\\"},{\\\"type\\\":\\\"string\\\",\\\"value\\\":\\\"|ELF.actions|\\\"},{\\\"type\\\":\\\"string\\\",\\\"value\\\":\\\"Action {action:function action() {\\\\n                STUDY.participantSettings = .extend(STUDY.participantSettings, {\\\\n                    participantID: {\\\\n                        seed: 100,\\\\n                        steps: 1,\\\\n                        max: 9000\\\\n                    },\\\\n                    participantIDFormat: \\\\\\\"0000-0000\\\\\\\",\\\\n                    participantNumberPortion: new ArrayRef([5, 9])\\\\n                });\\\\n                LF.StudyDesign.subjectAssignmentPhase = \\\\\\\"10\\\\\\\";\\\\n                STUDY.terminationPhase = \\\\\\\"999\\\\\\\";\\\\n                STUDY.studyPhase = new ObjectRef({\\\\n                    SCREENING: \\\\\\\"10\\\\\\\",\\\\n                    RANDOMIZATION: \\\\\\\"20\\\\\\\",\\\\n                    TREATMENT: \\\\\\\"30\\\\\\\",\\\\n                    FOLLOWUP: \\\\\\\"40\\\\\\\",\\\\n                    TERMINATION: \\\\\\\"999\\\\\\\"\\\\n                });\\\\n            }, ...} rejecting with TypeError: _.isArray is not a function. (In '.isArray(fromArray)', '.isArray' is undefined)\\\"},{\\\"type\\\":\\\"object\\\",\\\"objectId\\\":\\\"{\\\\\\\"injectedScriptId\\\\\\\":1,\\\\\\\"id\\\\\\\":22}\\\",\\\"subtype\\\":\\\"error\\\",\\\"className\\\":\\\"TypeError\\\",\\\"description\\\":\\\"TypeError: _.isArray is not a function. (In '.isArray(fromArray)', '.isArray' is undefined)\\\",\\\"preview\\\":{\\\"type\\\":\\\"object\\\",\\\"description\\\":\\\"TypeError: _.isArray is not a function. (In '.isArray(fromArray)', '.isArray' is undefined)\\\",\\\"lossless\\\":false,\\\"subtype\\\":\\\"error\\\",\\\"overflow\\\":false,\\\"properties\\\":[{\\\"name\\\":\\\"message\\\",\\\"type\\\":\\\"string\\\",\\\"value\\\":\\\".isArray is not a function. (In '.isArray(fromArray)', '.isArray' is undefined)\\\"}

Operating System

MacOS 13.6.6

Selenium version

appium-remote-debugger claims use of 4.19.0 for atoms.

What are the browser(s) and version(s) where you see this issue?

iPhone 12 - iOS 17.4.1 and iPhone 7 iOS 15.8.2

What are the browser driver(s) and version(s) where you see this issue?

appium-XCUITest-driver 7.17.6 (but have seen this issue since much earlier versions used with Appium 2.

Are you using Selenium Grid?

No

github-actions[bot] commented 4 days ago

@ahalbrock, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

titusfortner commented 4 days ago

Duplicate of #14181 and the underlying issue is #12549

ahalbrock commented 4 days ago

Agreed that this is likely a duplicate of https://github.com/SeleniumHQ/selenium/issues/14181 but https://github.com/SeleniumHQ/selenium/issues/12549 seems to be what caused the underlying issue to be apparent (swapping a "this" of { navigator: window.navigator, document: window.document } for just "window"). The issue seems to be the atoms deciding to write the function needing to be called to this. for no apparent reason before calling. If we're passing in window as "this" it doesn't seem advisable to overwrite chunks of in EVERY atom. I don't think we need "this. = mainFunc; return this._.apply(null, arguments)" when just "return mainFunc.apply(null, arguments)" seems to do the same job without potentially overwriting important properties of "window".

titusfortner commented 4 days ago

The code in Selenium right now is not what the Appium atoms are currently using. We reverted the code that was used to build those atoms.

If you have a code for #12549 that fixes the issue for Appium, please PR.