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.

Sign up for GitHub

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
Merged

fix: correctly disable animations in iframes for ios #813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/browser/existing-browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

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

// switchToParentFrame does not work in ios - https://github.com/appium/appium/issues/14882
await this._session.switchToFrame(null);
}
}
}

Expand Down
63 changes: 45 additions & 18 deletions test/src/browser/existing-browser.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

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

const iframeElement = { "element-12345": "67890_element_1" };
browser.publicAPI.findElements.withArgs("css selector", "iframe").resolves([iframeElement]);

await browser.prepareScreenshot(".selector", { disableAnimation: true });

assert.calledWith(clientBridge.call, "prepareScreenshot", [
".selector",
sinon.match({ disableAnimation: true }),
]);
assert.calledOnceWith(browser.publicAPI.switchToFrame, wdElement);
assert.calledWith(browser.publicAPI.switchToFrame, iframeElement);
assert.calledWith(clientBridge.call, "disableFrameAnimations");
});

Expand All @@ -760,15 +761,16 @@ describe("ExistingBrowser", () => {
it("should not disable animations if 'disableAnimation: false'", async () => {
const clientBridge = stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "webdriver" }));
const [wdElement] = await browser.publicAPI.findElements("css selector", ".some-selector");
const iframeElement = { "element-12345": "67890_element_1" };
browser.publicAPI.findElements.withArgs("css selector", "iframe").resolves([iframeElement]);

await browser.prepareScreenshot(".selector", { disableAnimation: false });

assert.neverCalledWith(clientBridge.call, "prepareScreenshot", [
".selector",
sinon.match({ disableAnimation: true }),
]);
assert.neverCalledWith(browser.publicAPI.switchToFrame, wdElement);
assert.neverCalledWith(browser.publicAPI.switchToFrame, iframeElement);
assert.neverCalledWith(clientBridge.call, "disableFrameAnimations");
});
});
Expand All @@ -792,27 +794,52 @@ describe("ExistingBrowser", () => {
assert.neverCalledWith(clientBridge.call, "cleanupFrameAnimations");
});

it("should cleanup animations in iframe if 'automationProtocol: webdriver'", async () => {
const clientBridge = stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "webdriver" }));
const [wdElement] = await browser.publicAPI.findElements("css selector", ".some-selector");
it("should not cleanup animations in iframe if 'automationProtocol: devtools'", async () => {
stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "devtools" }));

await browser.cleanupScreenshot({ disableAnimation: true });

assert.calledOnceWith(browser.publicAPI.switchToFrame, wdElement);
assert.calledWith(clientBridge.call, "cleanupFrameAnimations");
assert.callOrder(browser.publicAPI.switchToFrame, clientBridge.call);
assert.notCalled(browser.publicAPI.switchToFrame);
});

it("should not cleanup animations in iframe if 'automationProtocol: devtools'", async () => {
const clientBridge = stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "devtools" }));
const [wdElement] = await browser.publicAPI.findElements("css selector", ".some-selector");
describe("'automationProtocol: webdriver'", () => {
it("should cleanup animations in iframe", async () => {
const clientBridge = stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "webdriver" }));
const iframeElement = { "element-12345": "67890_element_1" };
browser.publicAPI.findElements.withArgs("css selector", "iframe").resolves([iframeElement]);

await browser.cleanupScreenshot({ disableAnimation: true });
await browser.cleanupScreenshot({ disableAnimation: true });

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 в него вообще не передается.

assert.calledWith(browser.publicAPI.switchToFrame, iframeElement);
assert.calledWith(clientBridge.call, "cleanupFrameAnimations");
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.

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

stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "webdriver" }));
const iframeElement = { "element-12345": "67890_element_1" };
browser.publicAPI.findElements.withArgs("css selector", "iframe").resolves([iframeElement]);

await browser.cleanupScreenshot({ disableAnimation: true });

assert.callOrder(
browser.publicAPI.switchToFrame.withArgs(iframeElement),
browser.publicAPI.switchToFrame.withArgs(null),
);
});

it("should not switch to any frame if there are no iframes on the page ", async () => {
stubClientBridge_();
const browser = await initBrowser_(mkBrowser_({ automationProtocol: "webdriver" }));
browser.publicAPI.findElements.withArgs("css selector", "iframe").resolves([]);

await browser.cleanupScreenshot({ disableAnimation: true });

assert.notCalled(browser.publicAPI.switchToFrame);
});
});
});

Expand Down
6 changes: 1 addition & 5 deletions test/src/browser/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ exports.mkSessionStub_ = () => {
click: sinon.stub().named("click").resolves(),
waitForExist: sinon.stub().named("waitForExist").resolves(),
};
const wdElement = {
"element-6066-11e4-a52e-4f735466cecf": "95777D6590AF653A2FD8EB0ADD20B333_element_1",
};

session.sessionId = "1234567890";
session.isW3C = false;
Expand All @@ -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.

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

session.findElements = sinon.stub().named("findElements").resolves([]);
session.switchToFrame = sinon.stub().named("switchToFrame").resolves();
session.switchToParentFrame = sinon.stub().named("switchToParentFrame").resolves();

session.addCommand = sinon.stub().callsFake((name, command, isElement) => {
const target = isElement ? wdioElement : session;
Expand Down
Loading