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

href attribute and min-width and max-width properties are not handled properly when using the media attribute #6669

Closed
camilamadronero opened this issue May 27, 2024 · 2 comments · Fixed by #6659
Assignees
Labels
lcp needs: grooming priority: medium Issues which are important, but no one will go out of business. severity: major Feature is not working as expected and no work around available type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@camilamadronero
Copy link

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

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

Describe the bug
The min-width and max-width properties, and the href attribute, are not handled properly when using the media attribute

To Reproduce
Steps to reproduce the behavior:

  1. Add this original markup to a page:
<figure class="figure-core-image post-featured single-image title-wrap">
	<picture class="core-image">
		<source type="image/webp" media='(max-width: 500px)' srcset="https://variance.pl/wp-content/uploads/2024/05/Kwiatowy-Ksiezyc-400x600.webp">
		<source type="image/webp" media='(min-width: 501px) and (max-width: 768px)' srcset="https://variance.pl/wp-content/uploads/2024/05/Kwiatowy-Ksiezyc-768x513.webp">
		<img class="skip-lazy" src="https://variance.pl/wp-content/uploads/2024/05/Kwiatowy-Ksiezyc-1348x900.webp" alt="Kwiatowy księżyc" title="Kwiatowy księżyc">
	</picture>
</figure>
  1. Wait until OCI is applied
  2. See the error, as the markup has turned into:
<link rel="preload" as="image" href="https://variance.pl/wp-content/uploads/2024/05/Kwiatowy-Ksiezyc-400x600.webp" media="(max-width: 500px)" fetchpriority="high">
<link rel="preload" as="image" href="https://variance.pl/wp-content/uploads/2024/05/Kwiatowy-Ksiezyc-768x513.webp" media="(min-width: 500.1px) and (min-width: 501px) and (max-width: 768px)" fetchpriority="high">
<link rel="preload" as="image" href="" media="(min-width: 768.1px)" fetchpriority="high">

Note how the (min-width: 501px) is duplicated, and how the href="" is empty

Expected behavior
To keep the right markup for the images.

Additional context
Slack: https://wp-media.slack.com/archives/C072P5EU5DF/p1716817705159419
Ticket: https://secure.helpscout.net/conversation/2608014439/493689/

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

@piotrbak
Copy link
Contributor

The second part is related to this one:
#6660

@piotrbak piotrbak added type: bug Indicates an unexpected problem or unintended behavior priority: medium Issues which are important, but no one will go out of business. severity: major Feature is not working as expected and no work around available lcp needs: grooming labels May 27, 2024
Miraeld added a commit that referenced this issue May 27, 2024
Fix the empty href & duplicated `min-width` with picture tag
@Miraeld
Copy link
Contributor

Miraeld commented May 27, 2024

Should be fixed with #6659

@Miraeld Miraeld self-assigned this May 27, 2024
@Miraeld Miraeld linked a pull request May 27, 2024 that will close this issue
19 tasks
@MathieuLamiot MathieuLamiot modified the milestones: 3.16.2, 3.16.1 Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lcp needs: grooming priority: medium Issues which are important, but no one will go out of business. severity: major Feature is not working as expected and no work around available type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants