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

WPML compatibility issue with LL BG image in specific scenarios #6224

Closed
Mai-Saad opened this issue Oct 13, 2023 · 3 comments · Fixed by #6280
Closed

WPML compatibility issue with LL BG image in specific scenarios #6224

Mai-Saad opened this issue Oct 13, 2023 · 3 comments · Fixed by #6280
Assignees
Labels
3rd party compatibility Issues related to 3rd party compatibility like theme, plugin or hosting effort: [XS] < 1 day of estimated development time module: lazyload background css images priority: medium Issues which are important, but no one will go out of business. 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

@Mai-Saad
Copy link

Mai-Saad commented Oct 13, 2023

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

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

Describe the bug
1- Console error for external absolute URL CSS files in scenario 1
2- A console error is there when trying to switch language in scenario 2

To Reproduce
Steps to reproduce the behavior:
Scenario1:
1- WPML activated on domain base setup
2- install /activate WPR, then cache LL BG template (containing absolute URL of CSS file)
3- enable LL BG I then access the sub language FE and hard refresh => We will have a console error for some CSS files

Scenario 2:
1- WPML is domain base, WPR LL BG image enabled
2- cache the page in both languages
3- WPML changed to a directory
4- access the main language page and click on language switch to sub-lang => console error

Expected behavior
No console error when accessing page/switching language with WPML while LL BG image is enabled

Screenshots
If applicable, add screenshots to help explain your problem.
scenario1:
274500362-c8974b91-3307-44df-bf5f-5a8091fc1779

scenario 2:
274507056-a2d5a5db-1f85-4ec9-ab08-f91fb78a1f7c

Additional context
Add any other context about the problem here.

  • Using Rocketlabs env
  • For scenario 2, if we clear the cache and then repeat step 4, we will have no console error => shall we automatically clear the cache when changing the WPML setup? @wp-media/productrocket

Acceptance Criteria (for WP Media team use only)
Clear instructions for developers, to be added before the grooming

  • No console error for main/sub language using domain base setup /directory setup and string setup
  • Switch language is working with no console error
  • Lazy load BG images working correctly with CDN on a regular and on a WPML environments
  • No regressions to regular environment LL BG images feature
@Mai-Saad Mai-Saad added type: bug Indicates an unexpected problem or unintended behavior 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. severity: moderate Feature isn't working as expected but has work around to get same value module: lazyload background css images labels Oct 13, 2023
@CrochetFeve0251
Copy link
Contributor

rocketlabs is using WPML and the issue is repoducable there.

@Miraeld Miraeld self-assigned this Nov 20, 2023
@CrochetFeve0251
Copy link
Contributor

Reproduce the issue

I cannot reproduce the second scenario.

Root issue

For the first scenario is due to that line

return str_replace( $url_host, sanitize_text_field( $_SERVER['HTTP_HOST'] ), $url ); // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.MissingUnslash
which add the new host url in all occurence in the url instead of just the domain leading to a extension added twice to the ll css generated file.

Scope a solution

We should change str_replace by preg_replace with the parameter one to stop at the first occurence.

Estimate effort

Effort XS

@CrochetFeve0251 CrochetFeve0251 added the effort: [XS] < 1 day of estimated development time label Nov 21, 2023
@Tabrisrp
Copy link
Contributor

Fix for first scenario looks good to me

@Miraeld Miraeld assigned Miraeld and unassigned Miraeld Nov 22, 2023
@Miraeld Miraeld mentioned this issue Nov 22, 2023
8 tasks
@piotrbak piotrbak added this to the 3.15.6 milestone Nov 29, 2023
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 module: lazyload background css images priority: medium Issues which are important, but no one will go out of business. 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
Development

Successfully merging a pull request may close this issue.

5 participants