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.

Sign up for GitHub

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

Jump to bottom

Update Delay JS script to latest version #4988

Closed
GeekPress opened this issue Apr 29, 2022 · 6 comments · Fixed by #5256
Closed

Update Delay JS script to latest version #4988

GeekPress opened this issue Apr 29, 2022 · 6 comments · Fixed by #5256
Assignees
@jeawhanlee
Labels
effort: [S] 1-2 days of estimated development time module: delay JS priority: high Issues which should be resolved as quickly as possible type: bug Indicates an unexpected problem or unintended behavior
Milestone
3.12.5

Comments

@GeekPress
Copy link
Contributor

Is your feature request related to a problem? Please describe.
We have 3 fixes left on the Delay JS script:

Describe the solution you'd like
Merge all PR to create a new version of the script.

Additional context
PR 24 needs some change inside WP Rocket too.
@GeekPress GeekPress added type: bug Indicates an unexpected problem or unintended behavior priority: high Issues which should be resolved as quickly as possible needs: grooming module: delay JS labels Apr 29, 2022
@GeekPress GeekPress added this to the 3.11.3 milestone Apr 29, 2022
@Tabrisrp
Copy link
Contributor

Scope a solution ✅

In addition to merging the 3 PRs and minifying the script, the following needs to happen in the Engine\Optimization\DelayJS\HTML:

In replace_scripts(), we also need to transform the src attribute to data-rocket-src if it is present in the attributes of the matched script.

The related automated tests will need to be updated to match the changes.

Estimate the effort ✅

Effort [S]

@Tabrisrp Tabrisrp added effort: [S] 1-2 days of estimated development time and removed needs: grooming labels May 10, 2022
@jeawhanlee jeawhanlee self-assigned this May 12, 2022
@jeawhanlee jeawhanlee mentioned this issue May 13, 2022
Merged
4 tasks
@Mai-Saad
Copy link
Contributor

We need to fix https://github.com/wp-media/delay-javascript-loading/issues/34

@Tabrisrp Tabrisrp modified the milestones: 3.11.3, 3.11.4 May 25, 2022
@gmetais
Copy link

gmetais commented Jun 6, 2022 •
edited by piotrbak
Loading

Hello,

I've added a commit (https://github.com/wp-media/delay-javascript-loading/pull/24/commits/ead18c3586b344c2da70f72aa0f61086ab51e0a9) on the branch fix-layout-slowness-2 that fixes the bug reported in https://github.com/wp-media/delay-javascript-loading/issues/34.

Can you please merge it with the rest?

(Thanks for the bug detection @Mai-Saad & @piotrbak !)

@piotrbak
Copy link
Contributor

piotrbak commented Jun 6, 2022

@jeawhanlee Could you update the script and the plugin's branch?

@gmetais Thank you! 🥇

@jeawhanlee
Copy link
Contributor

@piotrbak Ok

@vmanthos vmanthos self-assigned this Jun 8, 2022
@Tabrisrp Tabrisrp modified the milestone: 3.11.4 Jun 15, 2022
CrochetFeve0251 pushed a commit that referenced this issue Jun 27, 2022
@jeawhanlee
Closes #4988 Update delay js script & HTML markup (#5046)
69e2134 
* Transform src attribute to data-rocket-src

* Update fixture

* Remove double quote from needle

* Update delay js script

* Update needle for precise matching

* Added conditional to check that replacement is not added to script content

* Update fixtures

* Update fixtures

* Return Typehint

* Change to case-insensitive

* Fixes integrity attribute error on delay js script

* Check if script has unnecessary white space and treat as single script

* Updated fixtures for testcase with script tags having unneccessary white spaces

* Removed strict conditional check

* Script Update fix for (#34)

* Updated integration test

* Update delayjs script

* Update fixture

* Add data-rocket-src to script tags with comment as content

* Replace all src attribute of external script with data-rocket-src
engahmeds3ed added a commit that referenced this issue Jun 28, 2022
@engahmeds3ed
Revert "Closes #4988 Update delay js script & HTML markup (#5046)"
18dc947 
This reverts commit 69e2134.
@piotrbak piotrbak reopened this Jun 29, 2022
@Mai-Saad Mai-Saad removed this from the 3.11.4 milestone Jun 29, 2022
@jeawhanlee jeawhanlee mentioned this issue Jul 20, 2022
Merged
4 tasks
@vmanthos vmanthos removed their assignment Aug 26, 2022
@piotrbak piotrbak added this to the 3.12.2 milestone Sep 11, 2022
@piotrbak piotrbak added the status: blocked Issue or PR is blocked by external factor. label Sep 29, 2022
@piotrbak
Copy link
Contributor

Blocking the issue till we have an answer here:
https://github.com/wp-media/delay-javascript-loading/pull/32#issuecomment-1260857280

@piotrbak piotrbak modified the milestones: 3.12.2, 3.12.3 Oct 9, 2022
@piotrbak piotrbak removed the status: blocked Issue or PR is blocked by external factor. label Oct 31, 2022
@piotrbak piotrbak modified the milestones: 3.12.3, 3.12.4 Nov 6, 2022
Mai-Saad pushed a commit that referenced this issue Dec 14, 2022
@jeawhanlee
Closes #4988 Update delay js script & HTML markup (#5256)
172dbd7 
* Transform src attribute to data-rocket-src
* Update fixture
* Remove double quote from needle
* Update delay js script
* Update needle for precise matching
* Added conditional to check that replacement is not added to script content
* Update fixtures
* Update fixtures
* Return Typehint
* Change to case-insensitive
* Fixes integrity attribute error on delay js script
* Check if script has unnecessary white space and treat as single script
* Updated fixtures for testcase with script tags having unneccessary white spaces
* Removed strict conditional check
* Script Update fix for (#34)
* Updated integration test
* Update delayjs script
* Update fixture
* Add data-rocket-src to script tags with comment as content
* Replace all src attribute of external script with data-rocket-src
* Validate type attributes
* Updated allowed types
* Updated tests
* make quantifier lazy
* Resolved conflicts
* Updated method to replace script
* Updated fixtures
* Updated method
* Updated script and test
* Updated delayjs script
* refactor delayjs HTML code to use less number of regexes
* fix last test
* fix unit tests
* fix phpcs
* Remove invalid js mime type
* add drop resources table code back
Co-authored-by: Ahmed Saeed <eng.ahmeds3ed@gmail.com>
@piotrbak piotrbak modified the milestones: 3.12.4, 3.12.5 Dec 29, 2022
@Tabrisrp Tabrisrp mentioned this issue Jan 16, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeawhanlee jeawhanlee

Labels
effort: [S] 1-2 days of estimated development time module: delay JS priority: high Issues which should be resolved as quickly as possible type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Milestone
3.12.5
Development

Successfully merging a pull request may close this issue.

Closes #4988 Update delay js script & HTML markup wp-media/wp-rocket
7 participants
@vmanthos @gmetais @GeekPress @Tabrisrp @piotrbak @jeawhanlee @Mai-Saad