Add back-compat to old WP versions for new v2.2 features #6773

westonruter · 2021-12-08T05:21:18Z

Test new (and existing) plugin features in WordPress 4.9 and above. Add todos for fixing below:

Limit AMP integration on Add Plugins screen to WordPress 5.6+.
Limit AMP integration on Add Themes screen to WordPress 5.6+.
Limit plugin activation site scan to WordPress 5.6+.
Ensure Support screen works in 5.2+ and that it is hidden in older versions.
Add status=active parameter to /wp/v2/themes REST API calls for compat with WP<5.7. (See https://core.trac.wordpress.org/ticket/50152)
Limit the _fields requested form the plugins and themes endpoints to be just those which are needed.
Fix Settings screen loading in WP 4.9+.
Fix Onboarding Wizard loading in WP 4.9+.

Fixes #6775.
Closes #6771.

…PluginActivationSiteScan

… compat

westonruter · 2021-12-08T05:44:43Z

Add status=active parameter to /wp/v2/themes REST API calls for compat with WP<5.7. (See https://core.trac.wordpress.org/ticket/50152)

@delawski Is there a need to include non-active themes in the response? I suppose so since it seems that a parent theme is not included in the list if you add ?status=active. If so, 949eeb3 may need to be reverted and Site Scanning be made a WP 5.7+ required feature.

westonruter · 2021-12-08T05:53:33Z

It may be worthwhile to limit the queried themes and plugins to just those which are active, as if there are many themes and plugins installed will they all be included in the response? Actually, that brings to mind: what if there are hundreds of installed plugins and hundreds of installed themes? It seems that there is no pagination for either of the themes or plugins endpoints, so there should be a _fields parameter added to limit the fields to just the ones we need. In particular, I believe we should exclude fields like description.

includes/validation/class-amp-validation-manager.php

On WordPress 5.4 and older, the themes and plugins REST endpoints are not available. We use them in Site Scan for providing a source name, version, and author instead of a bare slug. Since this information is supplementary, we can fail silently if the endpoints are not available.

delawski · 2021-12-08T10:21:43Z

I've pushed a few commits that should fix the Site Scan on AMP Settings screen and in the Onboarding Wizard.

The scanner works now even though the/wp/v2/plugins and /wp/v2/themes REST paths are missing. We need the themes and plugins data just to provide additional information to the user. Even without it, we still have a slug which should be useful to a user:

Based on this assumption, I made the themes and plugins requests to fail silently in f57f25c.

Also, in 148d397 I've limited the fields we're requesting for themes and plugins. I haven't touched the pagination issue, though.

Lastly, with 3f3cb57 the Onboarding Wizard should work fine on WP 4.9.

src/Admin/SupportScreen.php

github-actions · 2021-12-08T16:59:00Z

Plugin builds for ff7e7b7 are ready 🛎️!

Download development build
Download production build

codecov · 2021-12-08T17:01:25Z

Codecov Report

Merging #6773 (bec8cbe) into develop (4cd08b2) will decrease coverage by 0.81%.
The diff coverage is 93.93%.

❗ Current head bec8cbe differs from pull request most recent head cff0d8f. Consider uploading reports for the commit cff0d8f to get more accurate results

@@              Coverage Diff              @@
##             develop    #6773      +/-   ##
=============================================
- Coverage      78.29%   77.47%   -0.82%     
- Complexity      6693     6700       +7     
=============================================
  Files            203      268      +65     
  Lines          20150    21356    +1206     
=============================================
+ Hits           15776    16546     +770     
- Misses          4374     4810     +436

Flag	Coverage Δ
javascript	`63.47% <ø> (?)`
php	`78.29% <93.93%> (+<0.01%)`	⬆️
unit	`78.29% <93.93%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Admin/AmpPlugins.php	`99.05% <50.00%> (-0.95%)`	⬇️
src/Admin/AmpThemes.php	`98.86% <50.00%> (-1.14%)`	⬇️
...cludes/validation/class-amp-validation-manager.php	`83.21% <100.00%> (+0.03%)`	⬆️
src/Admin/OnboardingWizardSubmenuPage.php	`91.12% <100.00%> (+0.45%)`	⬆️
src/Admin/OptionsMenu.php	`94.69% <100.00%> (+0.25%)`	⬆️
src/Admin/PluginActivationSiteScan.php	`100.00% <100.00%> (ø)`
src/Admin/SupportLink.php	`96.36% <100.00%> (+0.06%)`	⬆️
src/Admin/SupportScreen.php	`98.83% <100.00%> (+0.04%)`	⬆️
includes/sanitizers/class-amp-style-sanitizer.php	`87.45% <0.00%> (-0.20%)`	⬇️
assets/src/utils/use-async-error.js	`0.00% <0.00%> (ø)`
... and 64 more

westonruter · 2021-12-08T18:32:58Z

When testing Twenty Seventeen in WP 4.9, I was surprised to see a script error coming from comment-reply.min.js:

This is due to the id attribute being added to scripts starting with WP 5.5. So this is fixed with cff0d8f.

westonruter · 2021-12-08T21:32:24Z

In Wordpress 5.2.13 I see something weird where the REST API call to obtain the settings data is:

https://wordpress-stable.lndo.site/amp/v1/options?_locale=user

Note there is no /wp-json/ in the URL. This is resulting in the error screen being shown:

This is true of all the REST API URLs:

This error doesn't occur in WP 5.3.10, so there's something uniquely broken about WP 5.2.x.

westonruter · 2021-12-10T23:38:27Z

@delawski I think this was actually fixed by #6776 since without that chance I get errors in those 3 scripts:

If I revert your change in 3f3cb57 after having merged develop including #6776, I am no longer getting any errors and the Onboarding Wizard loads successfully in 4.9. Those three scripts are being included on the page.

This reverts commit 3f3cb57.

delawski · 2021-12-13T10:57:41Z

src/DevTools/UserAccess.php

+		// shown, and its the user interface which is particularly problematic on older versions of WP due to the
+		// JavaScript dependencies.
+		if ( ! $this->dependency_support->has_support() ) {
+			return false; // @codeCoverageIgnore


It seems that because of this check, it's impossible to toggle developer tools on WP 4.9-5.5. Is it intended?

I think we should just prevent users access to validation screens in WP<5.6.

@westonruter Actually, in light of #6775 (comment), do you think we should disable developer tools on WP<5.6, too? If yes, then we should hide the toggle here and also remove one of the steps in the Onboarding Wizard.

That's a good question. Developer tools access is broken down into two aspects:

Capability check: the users should never have access to DevTools (or the validation nonce)

Preference check: validation data should not be presented to the user.

In the case of a user being on an old WP install, the latter check is more appropriate because what we're wanting to do is to prevent displaying the validation UI to the user since it won't work properly. Technically such a user could still manually go to the validation screens if they know the URLs, but that is OK since they still have the capability and we aren't blocking their access.

So I think we can keep the step in the Onboarding Wizard. It's just we won't link to the Validated URLs screen.

I'm not sure if we're on the same page. Here's a recording of what I meant (notice that the response's payload always returns amp_dev_tools_enabled: false meaning the "Enable Developer Tools" toggle is useless):

Screen.Recording.2021-12-13.at.09.30.17.PM.mp4

The same applies to the Technical Background step of the Wizard.

Oh! I see what you're referring to.

I've addressed this in ff7e7b7 by hiding the toggle which appears on the settings screen as well as the toggle that appears on the user profile screen.

also remove one of the steps in the Onboarding Wizard.

@delawski Oh, I didn't do this part. If you would open a PR to address this, I'd appreciate it. It's consider this not to be a blocker for 2.2, since it only affects WP<5.6 (which granted is about a third of all installs).

Sure! This one seems to be easy: #6789.

Also remove invaid codeCoverageIgnore tags

dhaval-parekh · 2021-12-13T19:43:01Z

QA Passed.

✅ AMP Plugin/Theme is only highlighted in 5.6 and the newer versions of WordPress.

✅ Site scanning is disabled after plugin activation in the WordPress version prior to 5.6

✅ Support page is only available in 5.2 and the newer versions of WordPress.

✅ Did not find any issue on the "AMP Setting" screen and "Onboarding Wizard" in WordPress 4.9

dhaval-parekh

Changes in PR looks good to me.

See #6773 (comment)

delawski · 2021-12-15T10:38:41Z

QA Passed

✅ DevTools toggle is not rendered on WP<5.6

WP 4.9	WP 5.5	WP 5.6

Check dependency_support in is_needed for AmpPlugins, AmpThemes, and …

2a69d65

…PluginActivationSiteScan

westonruter added the P0 High priority label Dec 8, 2021

westonruter added this to the v2.2 milestone Dec 8, 2021

Add status=active parameter to /wp/v2/themes REST API call for WP<5.7…

949eeb3

… compat

westonruter assigned delawski Dec 8, 2021

Do not search for blocks when on WordPress 4.9 and older

7fa097e

delawski reviewed Dec 8, 2021

View reviewed changes

includes/validation/class-amp-validation-manager.php Show resolved Hide resolved

delawski added 4 commits December 8, 2021 10:32

Ensure Onboarding Wizard works on WordPress 4.9

3f3cb57

Fetch used plugin and theme fields only

148d397

Update message copy

6b553c4

dhaval-parekh reviewed Dec 8, 2021

View reviewed changes

src/Admin/SupportScreen.php Outdated Show resolved Hide resolved

dhaval-parekh added 3 commits December 8, 2021 19:30

Add minimum WordPress version check for Support page

c8a2d41

Update unit test cases

da361ae

Fix phpcs issues

bec8cbe

westonruter marked this pull request as ready for review December 8, 2021 16:54

westonruter requested a review from milindmore22 December 8, 2021 16:59

westonruter added 2 commits December 8, 2021 10:17

Improve theme/plugin unavailability messaging

15a2a0f

Ensure ID attribute is added to scripts in WP<5.5

cff0d8f

westonruter added 2 commits December 8, 2021 11:04

Harden injection of ID attribute into script loader tag

7345070

Fix regex pattern inversion

9360c3b

westonruter and others added 2 commits December 10, 2021 15:39

Revert "Ensure Onboarding Wizard works on WordPress 4.9"

721916a

This reverts commit 3f3cb57.

Add dependency_support as a requirement for AmpPlugins & AmpThemes

30b78fc

schlessera force-pushed the add/wp-back-compat branch from 517eb64 to 30b78fc Compare December 12, 2021 22:10

westonruter added 2 commits December 12, 2021 16:01

Add missing HasRequirements interface to PluginActivationSiteScan

3214f92

Add codeCoverageIgnore comments

942f91e

delawski previously requested changes Dec 13, 2021

View reviewed changes

westonruter requested a review from delawski December 13, 2021 17:20

westonruter added 5 commits December 13, 2021 09:50

Further harden regex in amp_ensure_id_attribute_on_script_loader_tag()

00ec54f

Restore wp_version in test tearDown method

24943e9

Run amp_ensure_id_attribute_on_script_loader_tag at earliest priority

e5c562b

Also remove invaid codeCoverageIgnore tags

Fix tests in WP 4.9..5.1

0dcdc77

Fix test_prepare_response_standard in WP<5.5

6d26e84

dhaval-parekh self-requested a review December 13, 2021 19:33

westonruter added 2 commits December 13, 2021 11:36

Add tests for AMP_Customizer_Design_Settings

f1c6208

Add tests for amp_bootstrap_plugin()

ccb9090

Fix test_init_customizer to account for Gutenberg being active

6dbd043

dhaval-parekh approved these changes Dec 13, 2021

View reviewed changes

westonruter added 2 commits December 13, 2021 15:19

Use remove_all_filters instead of remove_all_actions for purity sake

779104f

Hide DevTools toggles when dependency_support is absent

ff7e7b7

westonruter merged commit 27a2104 into develop Dec 14, 2021

westonruter deleted the add/wp-back-compat branch December 14, 2021 00:16

delawski mentioned this pull request Dec 14, 2021

Skip Technical Background step if there is no dependency support #6789

Merged

2 tasks

westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 15, 2021

milindmore22 mentioned this pull request Dec 16, 2021

AMP Standard: Admin menu item no longer shows in older versions of WP #6793

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add back-compat to old WP versions for new v2.2 features #6773

Add back-compat to old WP versions for new v2.2 features #6773

westonruter commented Dec 8, 2021 •

edited

Loading

westonruter commented Dec 8, 2021

westonruter commented Dec 8, 2021

delawski commented Dec 8, 2021

github-actions bot commented Dec 8, 2021 •

edited

Loading

codecov bot commented Dec 8, 2021 •

edited

Loading

westonruter commented Dec 8, 2021

westonruter commented Dec 8, 2021 •

edited

Loading

westonruter commented Dec 10, 2021

delawski Dec 13, 2021 •

edited

Loading

delawski Dec 13, 2021

westonruter Dec 13, 2021

delawski Dec 13, 2021

westonruter Dec 13, 2021

westonruter Dec 14, 2021

westonruter Dec 14, 2021

delawski Dec 14, 2021

dhaval-parekh commented Dec 13, 2021 •

edited

Loading

dhaval-parekh left a comment

delawski commented Dec 15, 2021

Add back-compat to old WP versions for new v2.2 features #6773

Add back-compat to old WP versions for new v2.2 features #6773

Conversation

westonruter commented Dec 8, 2021 • edited Loading

westonruter commented Dec 8, 2021

westonruter commented Dec 8, 2021

delawski commented Dec 8, 2021

github-actions bot commented Dec 8, 2021 • edited Loading

codecov bot commented Dec 8, 2021 • edited Loading

Codecov Report

westonruter commented Dec 8, 2021

westonruter commented Dec 8, 2021 • edited Loading

westonruter commented Dec 10, 2021

delawski Dec 13, 2021 • edited Loading

Choose a reason for hiding this comment

delawski Dec 13, 2021

Choose a reason for hiding this comment

westonruter Dec 13, 2021

Choose a reason for hiding this comment

delawski Dec 13, 2021

Choose a reason for hiding this comment

westonruter Dec 13, 2021

Choose a reason for hiding this comment

westonruter Dec 14, 2021

Choose a reason for hiding this comment

westonruter Dec 14, 2021

Choose a reason for hiding this comment

delawski Dec 14, 2021

Choose a reason for hiding this comment

dhaval-parekh commented Dec 13, 2021 • edited Loading

dhaval-parekh left a comment

Choose a reason for hiding this comment

delawski commented Dec 15, 2021

westonruter commented Dec 8, 2021 •

edited

Loading

github-actions bot commented Dec 8, 2021 •

edited

Loading

codecov bot commented Dec 8, 2021 •

edited

Loading

westonruter commented Dec 8, 2021 •

edited

Loading

delawski Dec 13, 2021 •

edited

Loading

dhaval-parekh commented Dec 13, 2021 •

edited

Loading