Skip to content
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

fix: correctly disable animations in iframes for ios #813

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

DudaGod
Copy link
Member

@DudaGod DudaGod commented Nov 29, 2023

What is done

Method switchToParentFrame is not supported in ios, so I used switchToFrame(null) which does the same thing.

@DudaGod DudaGod force-pushed the HERMIONE-1273.fix_disable_animation_ios branch from e21b841 to 70d16f8 Compare November 30, 2023 09:40
@@ -307,7 +307,10 @@ module.exports = class ExistingBrowser extends Browser {
await cb();
}
} finally {
await this._session.switchToParentFrame();
if (!_.isEmpty(iframes)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нет необходимости в вызове команды если айфреймов не было на странице.

@@ -731,15 +731,16 @@ describe("ExistingBrowser", () => {
it("should disable animations if 'disableAnimation: true' and 'automationProtocol: webdriver'", async () => {
const clientBridge = stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "webdriver" }));
const [wdElement] = await browser.publicAPI.findElements("css selector", ".some-selector");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот этот код был некорректный. Работал за счет того, что в utils застабан findElements и на любые переданные значение всегда возвращал какой-то элемент. В итоге тест становился неочевидным, так как непонятно по какому-то .some_selector-у находится элемент и еще он зачем-то передается в swithToFrame.

Поэтому данную логику изменил и сделал более очевидной.


assert.notCalled(browser.publicAPI.switchToFrame);
assert.neverCalledWith(clientBridge.call, "cleanupFrameAnimations", wdElement);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тоже невалидная строка была. В этом тесте clientBridge проверять не нужно и wdElement в него вообще не передается.

@@ -101,9 +98,8 @@ exports.mkSessionStub_ = () => {
session.mock = sinon.stub().named("mock").resolves(exports.mkMockStub_());
session.getWindowHandles = sinon.stub().named("getWindowHandles").resolves([]);
session.switchToWindow = sinon.stub().named("switchToWindow").resolves();
session.findElements = sinon.stub().named("findElements").resolves([wdElement]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Выгасил и возвращаю пустой массив. Если нужно переопределить возвращаемое значение, то это нужно сделать явно в тесте.

assert.callOrder(browser.publicAPI.switchToFrame, clientBridge.call);
});

it("should switch to parent frame after clean animations in iframe", async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил только этот тест и следующий за ним. Остальные просто пофиксил.

@DudaGod DudaGod changed the title fix: correctly disable animations in inframes for ios fix: correctly disable animations in iframes for ios Dec 19, 2023
@DudaGod DudaGod force-pushed the HERMIONE-1273.fix_disable_animation_ios branch from 70d16f8 to 5db5fa8 Compare December 19, 2023 10:49
@DudaGod DudaGod merged commit 3990aed into master Dec 19, 2023
2 checks passed
@DudaGod DudaGod deleted the HERMIONE-1273.fix_disable_animation_ios branch December 19, 2023 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants