Skip to content

Conversation

@CrochetFeve0251
Copy link
Contributor

@CrochetFeve0251 CrochetFeve0251 commented Mar 18, 2022

Description

Fix the problem which prevent the page from redirecting automatically to the ?amp=1 page without any user interaction.
This has been implemented by excluding wprRemoveCPCSS from Delay JS.
Fixes #4757

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Is the solution different from the one proposed during the grooming?

No the grooming was right

How Has This Been Tested?

  • Run tests locally
  • Check the behavior on my own testing website

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@CrochetFeve0251 CrochetFeve0251 added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement module: delay JS labels Mar 18, 2022
@CrochetFeve0251
Copy link
Contributor Author

CrochetFeve0251 commented Mar 18, 2022

@engahmeds3ed Should I change fixtures for this functionnality with the following script as it is external plugin: <script type="16b9331a55e54810d7a4cd33-text/javascript"> !function(){var e={};e.g=function(){if("object"==typeof globalThis)return globalThis;try{return this||new Function("return this")()}catch(e){if("object"==typeof window)return window}}(),function(n){let{ampUrl:t,isCustomizePreview:r,isAmpDevMode:o,noampQueryVarName:s,noampQueryVarValue:i,disabledStorageKey:a,mobileUserAgents:c,regexRegex:u}=n;if("undefined"==typeof sessionStorage)return;const d=new RegExp(u);if(!c.some((e=>{const n=e.match(d);return!(!n||!new RegExp(n[1],n[2]).test(navigator.userAgent))||navigator.userAgent.includes(e)})))return;e.g.addEventListener("DOMContentLoaded",(()=>{const e=document.getElementById("amp-mobile-version-switcher");if(!e)return;e.hidden=!1;const n=e.querySelector("a[href]");n&&n.addEventListener("click",(()=>{sessionStorage.removeItem(a)}))}));const g=o&&["paired-browsing-non-amp","paired-browsing-amp"].includes(window.name);if(sessionStorage.getItem(a)||r||g)return;const m=new URL(location.href),h=new URL(t);h.hash=m.hash,m.searchParams.has(s)&&i===m.searchParams.get(s)?sessionStorage.setItem(a,"1"):h.href!==m.href&&(window.stop(),location.replace(h.href))}({"ampUrl":"https:\/\/rocket.mitango.app\/?amp=1","noampQueryVarName":"noamp","noampQueryVarValue":"mobile","disabledStorageKey":"amp_mobile_redirect_disabled","mobileUserAgents":["Mobile","Android","Silk\/","Kindle","BlackBerry","Opera Mini","Opera Mobi"],"regexRegex":"^\\/((?:.|\n)+)\\/([i]*)$","isCustomizePreview":false,"isAmpDevMode":false})}(); </script>

…re by automated testing that this script is not deleted
@CrochetFeve0251
Copy link
Contributor Author

@engahmeds3ed I added the script to the fixtures

@engahmeds3ed
Copy link
Contributor

@CrochetFeve0251 it looks good to me.

@Tabrisrp we already have third party compatibility for for AMP, do u think it'd be better if we used the filter there to add this exclusion?

@remyperona
Copy link
Contributor

It could be cleaner that way indeed

@CrochetFeve0251
Copy link
Contributor Author

@engahmeds3ed I added the integration test

@remyperona remyperona changed the title Added amp-mobile-version-switcher to the HTML DelayJs exclusion array Fixes $4757 Exclude AMP redirection script from delay JS Mar 21, 2022
@remyperona remyperona changed the title Fixes $4757 Exclude AMP redirection script from delay JS Fixes #4757 Exclude AMP redirection script from delay JS Mar 21, 2022
CrochetFeve0251 and others added 5 commits March 21, 2022 14:29
Co-authored-by: Rémy Perona <remy@wp-rocket.me>
…ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>
…Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>
…ptimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>
…dParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>
@remyperona remyperona added this to the 3.11.1 milestone Mar 28, 2022
CrochetFeve0251 and others added 3 commits March 28, 2022 16:58
Co-authored-by: Rémy Perona <remy@wp-rocket.me>
…ptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>
…udeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>
@CrochetFeve0251
Copy link
Contributor Author

@Tabrisrp I made changes

@Mai-Saad Mai-Saad self-requested a review March 29, 2022 07:27
Copy link
Contributor

@Mai-Saad Mai-Saad left a comment

Choose a reason for hiding this comment

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

working as expected
testrail-report-364.pdf

@remyperona
Copy link
Contributor

@CrochetFeve0251 You can merge the PR using "squash and merge" method. 👍🏼

@CrochetFeve0251 CrochetFeve0251 merged commit 9f66438 into develop Apr 6, 2022
@CrochetFeve0251 CrochetFeve0251 deleted the fix/4757-add-amp-mobile-version-switcher-to-delayjs-html-excluded branch April 6, 2022 21:31
@CrochetFeve0251
Copy link
Contributor Author

CrochetFeve0251 commented Apr 6, 2022

@Tabrisrp just saw I forgot to merge that branch. Can I do the same with the others @Mai-Saad approuved?

@remyperona remyperona modified the milestones: 3.11.1, 3.11.0.3 Apr 11, 2022
CrochetFeve0251 added a commit that referenced this pull request May 2, 2022
* Fixes #4757 Exclude AMP redirection script from delay JS (#4827)

* Added amp-mobile-version-switcher to the HTML DelayJs exclusion array

* Added amp-mobile-version-switcher script the delayJs fixture to ensure by automated testing that this script is not deleted

* Reverted changes in DelayJS and fixtures corresponding to it.

* Added a call to the filter rocket_delay_js_exclusions in the AMP Subscriber to exclude amp-mobile-version-switcher

* Wrapped the filter rocket_delay_js_exclusions in the AMP verification to launch it only when it is activated

* Fixed code standards

* Added integration test for the exclude_script_from_delay_js method

* Update version in inc/ThirdParty/Plugins/Optimization/AMP.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Use configTestData instead of providerTestData tests/Integration/inc/ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Remove providerTestData function in tests/Integration/inc/ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Removed providerTestData mehod in tests/Unit/inc/ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Use configTestData instead of providerTestData in tests/Unit/inc/ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Update inc/ThirdParty/Plugins/Optimization/AMP.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Update tests/Unit/inc/ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Update tests/Integration/inc/ThirdParty/Plugins/Optimization/AMP/excludeScriptFromDelayJs.php

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Added Integration TestCase

Co-authored-by: Rémy Perona <remy@wp-rocket.me>

* Fixes #4804 Use absint values for width/height values when setting image dimensions of SVG files (#4805)

* Fixes #4527 Remove Rocket option to deactivate lazyload of iframes and YT videos when Autoptimize lazyload is enabled (#4889)

* Closes #4020 Delete metadata from the postmeta table during uninstall (#4855)

* Closes #4133 Clean feeds only if our cache feeds helper plugin is enabled (#4833)

* Fixes #4171 Exclude judge.me script from delay JS (#4839)

* Fixes #4032 Disabling heartbeat causing dependency error in query monitor (#4888)

* Prevent issues when parsing the HTML to apply optimizations (#4838)

* Fixes #4758 Exclude our remove CPCSS script from delay JS (#4824)

* Closes #4896 Update test suite (#4903)

Update our test suite to be cross-compatible with WP and PHPUnit versions

* Prevent PHP 8.1 deprecation notices (#4732)

* Flatsome preserver dynamic #image_ style (PR #4979)

Fixes #4971

* Preserve Health Center dynamic CSS selectors (PR #4976)

* Closes #4867 Update the deactivation form UI (PR #4880)

* Remove render class
* move deactivation form assets to deactivation intent class
* update classes instantiation
* add subscriber class
* update code for new deactivation intent form
* delete new option and transient on uninstall
* remove modal task
* remove old modal CSS
* remove old modal JS
* use micromodal for deactivation modal
* update the form HTML
* update the modal CSS
* update micromodal to v0.4.10
* return type hint
* add transition
* replace divs by list
* remove unneeded comment
* add missing semicolon
* update activate_safe_mode tests
* add unit tests
* add fixtures
* cast option to int
* update used hooks
* remove screen check
* update test and fixture
* integration tests
* fix PHPCS

* Fixes #4868 Apply CDN to used CSS on output instead of on saving (PR #4893)

* Moved the filter and removed the clean_used_css_with_cdn method
* Removed fixtures and integration tests from the method `clean_used_css_with_cdn`
* Added unit tests for treeshake function from UsedCSS controller
* Fixed code standards
* Removed quotes
* Fixed errors with the tests
* replicate magic isset declaration from parent class
* revert previous commit

Co-authored-by: Rémy Perona <remperona@gmail.com>

* Fix problem with action scheduler (PR #4965)

* Added fix
* Added fix standards
* Added check on pending
* Fixed search
* Added fix standards
* Fixed to conform to CR
* Fixed code standards
* Fixed code standards
* Changed `search` method usage to keep backward compatibility
* stop bailing out after cancel all actions

Co-authored-by: Ahmed Saeed <eng.ahmeds3ed@gmail.com>

* update plugin version

* Fixed failing test (#4986)

Co-authored-by: COQUARD Cyrille <18537428+CrochetFeve0251@users.noreply.github.com>
Co-authored-by: Rémy Perona <remy@wp-rocket.me>
Co-authored-by: Klemart3D <Klemart3D@users.noreply.github.com>
Co-authored-by: michael lee <38788055+jeawhanlee@users.noreply.github.com>
Co-authored-by: Rémy Perona <remperona@gmail.com>
Co-authored-by: Adame Dahmani <hello@adame.io>
Co-authored-by: Vasilis Manthos <vmanthos@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: delay JS type: enhancement Improvements that slightly enhance existing functionality and are fast to implement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exclude the script for AMP redirection from Delay JS

5 participants