LazyLoad Background CSS Images pseudo classes handling improvement

[piotrbak]

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
LazyLoad Background CSS Images is not handling correctly CSS background images that are added to specific selectors with pseudo classes:
-:after
-.class>:after
-.class&:after
-.class~:after

Single colon syntax is possible only for the following pseudo-elements:
::before, ::after, ::first-line, and ::first-letter
and they all should be covered. We should also cover single colon syntax for the following ones:
:hover :focus :active :visited :focus-within :focus-visible

For the double colon we should cover all of them with those special characters added just before:
-.class>::after
-.class&::after
-.class~::after

The root cause here is that .class>:after is a correct syntax while .class> is not. Our JS code is not able to find the pseudo classes and when removing them it reports wrong markup .class>.

To Reproduce
Steps to reproduce the behavior:

1. Add background image to any of the above classes and make sure the HTML matches
2. Enable Lazyload background CSS images
3. Open the website
4. Image is not loaded

Expected behavior
We should cover those scenarios, it might need a bit of r&d to find the best solution

Acceptance Criteria (for WP Media team use only)

1. We should be able to correctly find parent HTML elements related to pseudo classes
2. Lazy Load for background images should work for all the examples mentioned above
3. Lazy Load for background images should work for .class::after without regression
4. Lazy Load for background images should work for regular classes coming from internal style or CSS files

[CrochetFeve0251]

A possible fix is the following solution:

• Split the selector on each comma so we are sure now pseudo elements will be at the end of our selector.
• Remove the pseudo elements.
• Trim the selector to remove spaces.
• Check the last character and if it is one of the operator >&~ add * at the end of the string
• Re-concatenate our selector to get our final value (edited)

[CrochetFeve0251]

For that we gonna change the logic inside the MappingFormatter and change the logic from the remove_pseudo_classes method:

	protected function remove_pseudo_classes( string $selector ): string {
		$selectors = split($selector, ',');

                 $original_pseudo_elements = [
			':before',
			':after',
			':first-line',
			':first-letter',
		];

		/**
		 * Pseudo elements to remove from lazyload CSS selector.
		 *
		 * @param string[] Pseudo elements to remove.
		 */
		$pseudo_elements_to_remove = apply_filters( 'rocket_lazyload_css_ignored_pseudo_elements', $original_pseudo_elements );

		if ( ! is_array( $original_pseudo_elements ) ) {
			$pseudo_elements_to_remove = $original_pseudo_elements;
		}

     $selectors = array_map(function($selector) use ($pseudo_elements_to_remove) {
	           foreach ( $pseudo_elements_to_remove as $element ) {
			$selector = str_replace( $element, '', $selector );
		   }
                    if(in_array(substr($selector, -1), ['&','~','+'']) {
                       $selector .= '*';
                     }
                    return $selector;
            }, $selectors);



               $selector = implode($selectors, ',');

		if ( ! $selector ) {
			return 'body';
		}
		return (string) $selector;
	}

[engahmeds3ed]

I believe this needs R&D.

@CrochetFeve0251 the code u mentioned did u test it? or it's just an idea? I believe it should work.

[CrochetFeve0251]

I believe this needs R&D.

@CrochetFeve0251 the code u mentioned did u test it? or it's just an idea? I believe it should work.

It is just an idea to illustrate the logic

[remyperona]

I agree taking some time to do test with the solution and make sure it works would be the best

[MathieuLamiot]

@CrochetFeve0251
The code suggested here seems to handle well .class:before but not:
.class::before: I think it will remove :before, leaving .class: while we shoud get .class
.class>::after: I think it will leave .class>:, preventing to add * ; while we should get .class>*

We could list all variations (single and double colons) in original_pseudo_elements but it sounds a bit heavy and we could get funky results based on the order of the elements in the array ('::after', ':after') will not behave like (':after','::after')
Maybe we could keep something closer to the original development?

• Split the selector on commas
• preg_replace( '/::?[\w\-]+/', '', $selector ); so that we remove all pseudo elements with a single or double colons
• Add the &+> logic as suggested above.
• Merge back the results. @piotrbak Isn't there a risk that we have several time the same selector in the final result? Is that an issue? In which case, we have to merge with unicity or something like this.
  I'm thinking .class>::after, .class>::before could create the issue.

[MathieuLamiot]

@Tabrisrp is right, it should be easy to write this as a script an run it on some CSS files to confirm the behavior.

[piotrbak]

@MathieuLamiot The 'problem' would happen if class element is very big and has both .class>::after .class>::before defined. By the problem I mean that both images will be loaded, at the same point, when :after could have been loaded a bit later. I don't think this is a big deal.

[CrochetFeve0251]

Concerning the duplicated selector that can be handled quite easily using array_uniq but I didnt done so as it is a valid CSS selector.

Concerning the : : metaselector they were handled in the old logic but yes that would be better to handle them to.
However the issue is quite different as here we need to handle every class whereas for the : we need to handle only the given list as meta selectors like : nth-child are valid and should not be removed.

Concerning the script I really dont see the benefit from it as tests will force us to readapt any logic not valid.
However I think we should more brainstorm and search for edge cases in CSS that could break that logic to be sure it will be handled by the logic.

[MathieuLamiot]

Ah ok, then

• Split the selector on commas
• preg_replace( '/::[\w-]+/', '', $selector ); so that we remove all pseudo elements with a double colons
• str_replace with an array of single colon elements to handle. (with a filter as you suggested to remain flexible)
• Add the &+> logic as suggested above.
• Merge back the results.

We need to handle double colon first.