-
-
Notifications
You must be signed in to change notification settings - Fork 410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add pageLoadStrategy for Safari/WebView #2411
Conversation
@@ -380,6 +380,10 @@ const desiredCapConstraints = /** @type {const} */ ({ | |||
appTimeZone: { | |||
isString: true, | |||
}, | |||
pageLoadStrategy: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it should be already defined in the base driver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I thought so, but it was used to check appium prefix stuff only:
With current master:
[XCUITestDriver@b618] The following provided capabilities were not recognized by this driver:
[XCUITestDriver@b618] pageLoadStrategy
[XCUITestDriver@b618] The desired capabilities include neither an app nor a bundleId. WebDriverAgent will be started without the default app
[XCUITestDriver@b618] Session created with session id: 609c241a-fcf6-44a6-be40-c5eff0cde7fe
Other standard caps which are used by this driver also defined in this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird...
I would expect all standard W3C caps to be defined there. I can also observe corresponding typedefs present:
https://github.com/appium/appium/blob/master/packages/types/lib/standard-caps.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without this line, building this.caps.pageLoadStrategy
in lib/commands/context.js also failed as:
lib/commands/context.js:463:37 - error TS2339: Property 'pageLoadStrategy' does not exist on type 'DriverCaps<{ readonly platformName: { readonly presence: true; readonly isString: true; readonly inclusionCaseInsensitive: readonly ["iOS", "tvOS"]; }; readonly browserName: { readonly isString: true; }; ... 119 more ...; readonly appTimeZone: { ...; }; }>'.
463 pageLoadStrategy: this.caps.pageLoadStrategy,
~~~~~~~~~~~~~~~~
Hm, then something might not work expectedly in the base driver side...?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like there is a bug in typedefs. Not sure where exactly yet...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created for future investigation appium/appium#20273 when we have time...
## [7.18.0](v7.17.6...v7.18.0) (2024-06-20) ### Features * add pageLoadStrategy for Safari/WebView ([#2411](#2411)) ([2517bf7](2517bf7))
🎉 This PR is included in version 7.18.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Based on appium/appium-remote-debugger#385 idea.