fix: tell a native session from a browser one in every adapter - #375
fix: tell a native session from a browser one in every adapter#375vishnuv688 wants to merge 11 commits into
Conversation
|
| if (!deviceFromCapabilities(caps) && !namesAnAutomation(caps)) { | ||
| return false | ||
| } | ||
| return !deepCapString(caps, 'browserName') |
There was a problem hiding this comment.
Hybrid contexts lose DOM capture
Hybrid Appium sessions are classified for their entire lifetime from startup capabilities, even though their active context can switch between native and webview. When a native-capability session enters a webview, this predicate remains true, so the new guards in the Selenium, Nightwatch, and Python adapters keep skipping collector injection, DOM drains, performance reads, and element snapshots. The webview portion of the trace therefore has no DOM evidence. Document availability needs to follow the current runtime context for hybrid sessions rather than remain fixed from capabilities.
There was a problem hiding this comment.
Real, and deliberately so — the predicate's docstring states this residual, and it is carried over from the service's isNativeAppSession already on main (#371), not introduced here.
The behaviour change for Selenium/Nightwatch/Python is accurate though, and worth stating plainly: those three gated nothing before, so a hybrid session's webview portion was captured by accident and now is not.
Not fixing it in this PR. Following the runtime context means caching a getContext() read and invalidating it on context switches across core and three adapters — outside #372's scope, and unverifiable without a hybrid app on a real device. The cheap-direction argument also cuts the other way: treating a document-less session as web costs the service's 8 s-timeout probe poll per action, which is #351.
Tracked in #376.
What & why
Every adapter can now tell a native session from a browser one. Until now only the WDIO service could: the predicate read
browser.isMobile/isAndroid/isIOS, which are WDIO runtime flags that Selenium'sWebDriver, Nightwatch'sbrowserand the Python driver do not have. So Selenium and Nightwatch ran their DOM drain, their collector injection and their page-script probes against a native app anyway — the same wasted round trips andMethod is not implementederrors the service stopped emitting in #371 — and the Python adapter readwindow.innerWidthon a session with no window.The fact now has one reader,
isNativeAppSessioninshared, which asks the capabilities every adapter already publishes rather than a driver flag. It keys on whether the session named a browser, because a device alone does not answer the question — an Appium session driving Chrome or Safari runs on a phone and has a real page — and it reads bothplatformNameandbrowserNameone level into vendor options, since a device cloud commonly states them only inside its own bag.Gated per adapter: Selenium's
captureTrace,injectScript,reinjectIfNavigatedand its performance read (whose 500 ms settle was being spent to reach a document that does not exist); Nightwatch'scaptureTrace,injectScript,anchorAfterNavigationand its own performance read; Python's collector, its performance read and its viewport, which now measures the device window rather than asking a page that isn't there.The densest of these is the per-action snapshot, and all three adapters were paying it: two injected scripts plus
urlandtitle, on every action. Those four now drop out on a native session while the screenshot — the one probe a native app does serve — is still taken, so the trace keeps its per-action frames. Screenshots andmanage().logs()are deliberately left alone: Appium serves both, and logcat arrives through the second, so gating them would lose data rather than save a failed call.Two pre-existing bugs fell out of the work, both from one root: Selenium published its capabilities as selenium-webdriver's
Capabilitiesinstance, whose data lives in a private Map withserializeexposed only under a Symbol — so the string-keyedserialize?.()the adapter called returnedundefinedand the instance reached the dashboard as{"map_":{}}. Every Selenium trace therefore carried nodeviceand a guessed browser name, and the capabilities pane was empty. It is now flattened through the class's ownkeys()/get(), which is also what makes the new guards work at all, since they read that bag. Reading the device out of vendor options fixes the same field for a cloud session, which previously read as desktop and reached the player framed as a browser window rather than a phone.The player's mobile layout was also still WDIO-only in live mode: it gates on
metadata.device, and only the service derived one before sending. The app now derives it from the capabilities when the adapter sent none, at the single ingestion point every live message passes through. A device the adapter did send wins. Trace mode was already correct — the exporter derives it on the way into the zip.Advances #372. Not closing it: two checklist items remain, both below.
Type of change
Packages touched
shared(types and contracts)core(framework-agnostic capture/reporting)elements(published element/snapshot API —@wdio/elements)service(WebdriverIO adapter)nightwatch-devtools(Nightwatch adapter)selenium-devtools(Selenium adapter)selenium-devtools-py(Selenium Python adapter)backend(server)app(UI)script(page-injected runtime)trace(Trace mode)Notes for reviewers
Why the shared logic is in
sharedand notcore. #372 proposedcore, and this lands insharedinstead. The predicate is a pure read over a capability bag — no driver, no framework hook, no capture session — andsharedalready owns the neighbouring narrowing (isNativePlatform,deviceFromCapabilities) that thetraceexporter calls. Putting it incorewould have split one question across two packages and put it out of reach of anything that may not importcore. It stays framework-agnostic either way;sharedis the layer more consumers can reach.Why the guards read metadata rather than the driver.
SessionCapturerBaseexposes it asisNativeAppSession, resolved from the metadata the adapter has already set. That indirection is not decoration: Selenium's owngetCapabilities()is async, and a guard cannot await it at the point it has to decide.Why the guards are inside the guarded methods, not at the call sites. This is #350's finding applied: two of four call sites asked and two forgot. Selenium's drain has three call sites and Nightwatch's has four.
Known gaps, both tracked, neither introduced here:
isNativeAppSessionanswers from startup capabilities and is never revisited, so a hybrid app switched into a webview context keeps being treated as native and its webview portion carries no DOM. Raised in review on this PR. Not fixed here because following the runtime context means caching agetContext()read and invalidating it on context switches across core and three adapters, and because the opposite default is the expensive one — treating a document-less session as web costs the service's 8 s-timeout probe poll per action (Native mobile trace mode costs 40-60s per run in per-action probes #351).A native session also still gets no accessibility tree: deriving one from page source is a capture feature the service has and the other three do not.
Verification
CI cannot cover any of this — it needs a device.
pnpm lint,pnpm build,pnpm test(2361 tests) andpnpm test:uiare green on CI; the native paths were exercised locally against a real Android emulator through a per-adapter Appium example, driving the device's own Settings app so no.apkor credentials are involved.The mobile examples themselves are not in this PR — they are #372's remaining checklist item and land separately. That is the second reason this PR does not close #372.
One finding from that exercise is worth flagging because it is a live blocker rather than a gap in this change: a mobile web Appium session deadlocks with the service attached (#374) —
beforeCommandawaits page-side calls from inside the hook wrapping the command being issued, Appium serialises commands per session, so the wrapped command never reaches the browser. Measured at 6 m 13 s of timeouts against 1.6 s with the service removed. Native app sessions are unaffected, precisely becauseisNativeAppSessionmakes those same calls no-ops — which is the change in this PR.Screenshots / recordings
n/a — no UI change beyond live-mode device derivation, which selects the existing mobile layout rather than adding one.