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

Problem with handling two identical DOM elements with Lazyload Background CSS Images #6158

Closed
piotrbak opened this issue Sep 11, 2023 · 4 comments · Fixed by #6167
Closed
Assignees
Labels
effort: [S] 1-2 days of estimated development time module: lazyload background css images needs: documentation Issues which need to create or update a documentation priority: high Issues which should be resolved as quickly as possible severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@piotrbak
Copy link
Contributor

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

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

Describe the bug
When we're dealing with two identical DOM elements with backgrounds, our script is not handling this properly.

To Reproduce
Steps to reproduce the behavior:

  1. Use the latest WP Rocket version
  2. Use the following markup https://pastebin.com/VnYkwUgv
  3. Enable the Lazyload Background CSS Images feature
  4. Visit the page in incognito mode
  5. Images are not loaded with variable not defined message

Additional Context
This is not regression when comparing to #6150

Acceptance Criteria (for WP Media team use only)

  1. We should handle two or more identical DOM elements having background images
  2. We should not introduce regression compared to the previous state. The templates that could be used for e2e tests are available here and here.
@piotrbak piotrbak added type: bug Indicates an unexpected problem or unintended behavior priority: high Issues which should be resolved as quickly as possible needs: grooming severity: moderate Feature isn't working as expected but has work around to get same value module: lazyload background css images labels Sep 11, 2023
@CrochetFeve0251
Copy link
Contributor

Reproduce the issue

I was able to reproduce the issue when having two different selectors on the same DOM node.

Root cause

The bug occurs due to the first selector setting the data-rocket-lazy-bg to loaded preventing the second selector from loading its CSS variable.

Scope a solution

To fix that we will have to make the attribute unique to the pair in question and for this we gonna use its hash to give it this form:
data-rocket-lazy-bg-{hash}

For that we will have to modify two parts:

  • The JSON formatter to make it load the hash.
  • The script to make it add the hash to the attribute.

To add the field to the formatter nothing more easy just add a new field hash with the value $data['hash'] inside format.

To change the js to use the hash we need to add the hash value from the pair to the attribute on theses lines:

Estimate effort

Effort S

@CrochetFeve0251 CrochetFeve0251 added effort: [S] 1-2 days of estimated development time and removed needs: grooming labels Sep 11, 2023
@Tabrisrp
Copy link
Contributor

looks good to me

@MathieuLamiot
Copy link
Contributor

@CrochetFeve0251 Can this task be handled by someone else in the PHP team or is it much more efficient if you do it?

@CrochetFeve0251
Copy link
Contributor

@CrochetFeve0251 Can this task be handled by someone else in the PHP team or is it much more efficient if you do it?

I think anyone could handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [S] 1-2 days of estimated development time module: lazyload background css images needs: documentation Issues which need to create or update a documentation priority: high Issues which should be resolved as quickly as possible severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
6 participants