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

Exclude the script for AMP redirection from Delay JS #4757

Closed
4 tasks
Mai-Saad opened this issue Feb 22, 2022 · 3 comments · Fixed by #4827
Closed
4 tasks

Exclude the script for AMP redirection from Delay JS #4757

Mai-Saad opened this issue Feb 22, 2022 · 3 comments · Fixed by #4827
Assignees
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting effort: [XS] < 1 day of estimated development time good first issue Indicates a good issue for first-time contributors module: delay JS priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@Mai-Saad
Copy link
Contributor

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version => 3.10.8
  • Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug
The script which redirects mobile pages to the amp is delayed

To Reproduce

  • Cache for mobile devices is enabled
  • Delay JS is enabled
  • Amp redirect is enabled

Steps to reproduce the behavior:

  1. Open the home page in mobile view
  2. redirect to ?amp does not occur till user interaction

Expected behavior
Amp redirect occurs without user interaction

Additional context
Add any other context about the problem here.

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@Mai-Saad Mai-Saad added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement 3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting priority: medium Issues which are important, but no one will go out of business. module: delay JS labels Feb 22, 2022
@Mai-Saad Mai-Saad changed the title Exclude the script for AMP redirect from Delay JS Exclude the script for AMP redirection from Delay JS Feb 22, 2022
@GeekPress
Copy link
Contributor

@Mai-Saad Which JS file or inline script must be excluded from Delay JS to have the redirection without user interaction?

@Mai-Saad
Copy link
Contributor Author

Mai-Saad commented Mar 4, 2022

@GeekPress this is the inline script (using AMP v2.2.1)

<script type="rocketlazyloadscript">
 !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:\/\/new.rocketlabsqa.ovh\/?amp","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>

excluding amp-mobile-version-switcher works fine for both standard and transitional mode

@remyperona
Copy link
Contributor

Scope a solution ✅

Add amp-mobile-version-switcher to the DelayJS\HTML::$excluded array

Estimate the effort ✅

Effort [XS]

@remyperona remyperona added the effort: [XS] < 1 day of estimated development time label Mar 7, 2022
@remyperona remyperona added the good first issue Indicates a good issue for first-time contributors label Mar 14, 2022
@CrochetFeve0251 CrochetFeve0251 self-assigned this Mar 18, 2022
@piotrbak piotrbak added this to the 3.11.1 milestone Mar 28, 2022
CrochetFeve0251 added a commit that referenced this issue Apr 6, 2022
* 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 <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

* Added Integration TestCase

Co-authored-by: Rémy Perona <[email protected]>
@remyperona remyperona modified the milestones: 3.11.1, 3.11.0.3 Apr 11, 2022
@remyperona remyperona mentioned this issue Apr 11, 2022
CrochetFeve0251 added a commit that referenced this issue 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 <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

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

Co-authored-by: Rémy Perona <[email protected]>

* Added Integration TestCase

Co-authored-by: Rémy Perona <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* update plugin version

* Fixed failing test (#4986)

Co-authored-by: COQUARD Cyrille <[email protected]>
Co-authored-by: Rémy Perona <[email protected]>
Co-authored-by: Klemart3D <[email protected]>
Co-authored-by: michael lee <[email protected]>
Co-authored-by: Rémy Perona <[email protected]>
Co-authored-by: Adame Dahmani <[email protected]>
Co-authored-by: Vasilis Manthos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting effort: [XS] < 1 day of estimated development time good first issue Indicates a good issue for first-time contributors module: delay JS priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants