Skip to content
This repository has been archived by the owner on Aug 13, 2023. It is now read-only.

Revert 2836 - Add AV embed timeout handling to MP #2991

Merged
merged 9 commits into from
Jan 23, 2020
Merged

Conversation

simonsinclair
Copy link
Contributor

@simonsinclair simonsinclair commented Jan 22, 2020

Overall change: This is a manual revert of #2836, which added AV embed timeout handling to Media Player. We had to revert this due to a strange interaction with Simorgh's e2e tests, which could be related to a bug in Cypress. See below for additional context. Until we can offer this issue more time, this functionality will be reverted.

Code changes:

Additional context:

  • Attaching an on load event handler to Canonical MP iframe causes the first e2e test that accesses the iframe's contentWindow property to fail. Subsequent tests will pass. It does not matter on the order of these tests - the first one will always fail.
  • A possible related open issue with Cypress: Handle stale load events that leak from previous windows  cypress-io/cypress#4973.
  • When we can afford more time to this problem, it might be worth replicating this issue in a slim repository that mimics our setup and use it to raise a bug report with Cypress.

  • I have assigned myself to this PR and the corresponding issues
  • Automated jest tests added (for new features) or updated (for existing features)
  • This PR requires manual testing

@simonsinclair simonsinclair added the ws-articles Tasks for the WS Articles Team label Jan 22, 2020
@simonsinclair simonsinclair self-assigned this Jan 22, 2020
@simonsinclair simonsinclair changed the title Revert #2836 - Add AV embed timeout handling to Media Player Revert 2836 - Add AV embed timeout handling to MP Jan 22, 2020
Copy link
Contributor

@j-pendlebury j-pendlebury left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@amywalkerdev amywalkerdev merged commit 0054bcc into latest Jan 23, 2020
@amywalkerdev amywalkerdev deleted the revert-2836 branch January 23, 2020 09:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ws-articles Tasks for the WS Articles Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants