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

Paired browsing JS fails to load in WordPress 4.9 #3932

Closed
westonruter opened this issue Dec 16, 2019 · 3 comments · Fixed by #3963
Closed

Paired browsing JS fails to load in WordPress 4.9 #3932

westonruter opened this issue Dec 16, 2019 · 3 comments · Fixed by #3963
Labels
Bug Something isn't working
Milestone

Comments

@westonruter
Copy link
Member

Bug Description

Originally reported in #3928 (comment).

There is a conflict in WordPress 4.9 due to the amp-paired-browsing-client not being printed: https://travis-ci.org/ampproject/amp-wp/jobs/625330248#L1062-L1068

I believe this is due to the fact that amp-paired-browsing-client depends on both wp-dom-ready and wp-polyfill but only the former is being polyfilled for WordPress 4.9:

amp-wp/webpack.config.js

Lines 261 to 263 in 80e70e3

'wp-i18n': './assets/src/polyfills/wp-i18n.js',
'wp-dom-ready': './assets/src/polyfills/wp-dom-ready.js',
'wp-server-side-render': './assets/src/polyfills/wp-server-side-render.js',

In other words, there is no polyfill for the polyfills.

Three options I see here:

  1. Increase minimum WP version to 5.0 and eliminate these pre-Gutenberg polyfills altogether.
  2. Polyfill the polyfills.
  3. Prevent paired browsing from being available in WordPress 4.9.

I've gone ahead and removed the test case in 64a8051, but this will need to be fixed for v1.5 in a separate PR. That PR can revert this commit to re-add those tests.

Expected Behaviour

Paired browsing should either be not available or the minimum version of WordPress should be bumped.

Steps to reproduce

Try to access paired browsing in WordPress 4.9.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter westonruter added this to the v1.5 milestone Dec 16, 2019
@swissspidy swissspidy added the Bug Something isn't working label Dec 16, 2019
@pierlon
Copy link
Contributor

pierlon commented Dec 16, 2019

I believe adding a polyfill for the polyfills would be the safest option here, but increasing the minimum WP version to 5.0 is also an interesting option to investigate. If information is available to analyze the current installs of the plugin by WP version that would make the decision of increasing said version much easier.

@csossi
Copy link

csossi commented Feb 13, 2020

any QA instructions for me here, @pierlon ?

@pierlon
Copy link
Contributor

pierlon commented Feb 13, 2020

Developer QA done by @swissspidy so this can be moved to Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants