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

Vite adds spaces around commas in image url within srcset (HTML files) #16077

Closed
7 tasks done
caseycarroll opened this issue Mar 2, 2024 · 4 comments · Fixed by #16081
Closed
7 tasks done

Vite adds spaces around commas in image url within srcset (HTML files) #16077

caseycarroll opened this issue Mar 2, 2024 · 4 comments · Fixed by #16081
Labels
feat: html p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@caseycarroll
Copy link
Contributor

caseycarroll commented Mar 2, 2024

Describe the bug

Vite appears to add spaces around commas when a comma is present in a srcset property. This only occurs in HTML files.

If my index.html file contains

    <img 
       srcset="https://www.rei.com/dam/18160142_homepage-lead-member-rewards-xs_web_lg.jpeg?im=Resize,width=800 800w"
       src="https://www.rei.com/dam/18160142_homepage-lead-member-rewards-xs_web_lg.jpeg"
    />

Vite outputs

<img srcset="https://www.rei.com/dam/18160142_homepage-lead-member-rewards-xs_web_lg.jpeg?im=Resize , width=800 800w" src="https://www.rei.com/dam/18160142_homepage-lead-member-rewards-xs_web_lg.jpeg">

Take note of the space added to the image url around the comma in the query parameter. ?im=Resize , width=800

The browser reads width=800 as the entire image url and attempts to request /width=800 which is nonexistent.

Screenshot 2024-03-02 004053

Reproduction

Code Sandbox

Steps to reproduce

reproduceable with vite vite preview and vite build

System Info

System:
    OS: Windows 11 10.0.22631
    CPU: (6) x64 Intel(R) Core(TM) i5-8600K CPU @ 3.60GHz
    Memory: 8.79 GB / 15.93 GB
  Binaries:
    Node: 21.6.1 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.21 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 10.2.4 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Chromium (122.0.2365.59)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    vite: ^5.1.4 => 5.1.4

Used Package Manager

npm

Logs

Browser Logs ```shell Uncaught TypeError: Cannot set properties of null (setting 'innerHTML') at main.ts:6:49 ```
Vite Logs ```shell vite:resolve 0.64ms /width=800 -> null +5s vite:html-fallback Rewriting GET /width=800 to /index.html +228ms ```

Validations

@sapphi-red sapphi-red added feat: html p3-minor-bug An edge case that only affects very specific usage (priority) labels Mar 2, 2024
@caseycarroll
Copy link
Contributor Author

I believe the bug lives in this line of splitSrcSet:

splitIndex = cleanedSrcs.indexOf(',', startIndex)

I will try to make a contribution this weekend, but anyone else should feel free to jump on it.

@rarous
Copy link
Contributor

rarous commented Mar 31, 2024

Hi, this is still broken for URLs with commas in path, eg. https://res.cloudinary.com/hackercamp/image/upload/dpr_auto,f_auto,fl_progressive,q_auto,w_100/v1/www.hackercamp.cz/faq-ubytko-3?_a=BAMABmTE0

@patak-dev
Copy link
Member

Hey @rarous, please create a new issue with a minimal reproduction so we can properly track your report

@rarous
Copy link
Contributor

rarous commented Mar 31, 2024

Ok, my case is described in #16323

@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: html p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
4 participants