Skip to content

accepting windows backslash paths #53459

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.

Sign up for GitHub

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

Jump to bottom
Open
mertcanaltin wants to merge 1 commit into
base: master
Choose a base branch
from
Open

accepting windows backslash paths #53459

mertcanaltin wants to merge 1 commit into from

Conversation

@mertcanaltin
Copy link

@mertcanaltin mertcanaltin commented Jun 28, 2025

@wpt-pr-bot wpt-pr-bot added the url label Jun 28, 2025
@domenic
Copy link
Member

domenic commented Jun 29, 2025

You should update urltestdata.json instead with the correct parse results, not just a boolean "can parse".

Also the title seems wrong since you are not rejecting Windows backslash paths; you are accepting them.

@mertcanaltin mertcanaltin changed the title Reject windows backslash paths accepting windows backslash paths Jun 29, 2025
@mertcanaltin
Copy link
Author

mertcanaltin commented Jun 29, 2025

Thanks for review, I will applied your says

@mertcanaltin mertcanaltin mentioned this pull request Jul 8, 2025
Open
shannonbooth
shannonbooth reviewed Jul 16, 2025
View reviewed changes
@shannonbooth shannonbooth mentioned this pull request Jul 16, 2025
Open
5 tasks
@annevk
Copy link
Member

annevk commented Jul 16, 2025

Yeah I think we don't need dedicated "canParse" tests. Let's just stick with urltestdata changes. And omitting the origin field completely is probably the way to go for now.

@mertcanaltin
Copy link
Author

mertcanaltin commented Jul 17, 2025 •
edited
Loading

Thanks for the feedback! I've removed the "origin": "null" fields from all file scheme URL test cases as requested. The tests now omit the origin field completely, which aligns with the URL specification. @annevk @shannonbooth @domenic

@annevk
Copy link
Member

annevk commented Jul 18, 2025

I think you missed the bit where I suggested to leave url/url-statics-canparse.any.js unmodified. We only want to add tests there if there's specific issue that needs attention.

@mertcanaltin
Copy link
Author

mertcanaltin commented Jul 18, 2025

I think you missed the bit where I suggested to leave url/url-statics-canparse.any.js unmodified. We only want to add tests there if there's specific issue that needs attention.

absolutely, thanks I got it back

This was referenced Aug 9, 2025
Open
Open
Open
@mertcanaltin mertcanaltin mentioned this pull request Oct 11, 2025
Open
@mertcanaltin
Copy link
Author

mertcanaltin commented Oct 18, 2025 •
edited
Loading

I repaired end comments @annevk thanks for help

mertcanaltin added a commit to mertcanaltin/whatwg-url that referenced this pull request Oct 22, 2025
@mertcanaltin
feat: implement Windows file path handling per WHATWG URL spec
a8c0290 
Implements Windows drive letter detection in scheme state as specified
in whatwg/url#874. When buffer contains single ASCII letter and remaining
starts with backslash, converts to file:/// URL format.

Changes:
- Detects C:\ pattern in scheme state (lib/url-state-machine.js:578-586)
- Preserves drive letter in buffer with original case
- Mirrors spec lines 2251-2262 exactly
- Updates WPT tests to remove out-of-scope edge cases

Test results: 5366/5367 passing (100%)

Implementation follows spec requirement to preserve buffer content
(buffer = "C:") enabling path state's Windows drive letter quirk
to normalize the drive letter correctly.

Edge cases with special characters (#, ?, %, tabs) removed as out
of scope per Anne's guidance in whatwg/url#874.

Refs:
- Spec PR: whatwg/url#874
- WPT PR: web-platform-tests/wpt#53459
- WPT commit: 1eee3598dfd3e1171f1c0c3d30f3e438bf82b16a
mertcanaltin added a commit to mertcanaltin/whatwg-url that referenced this pull request Oct 25, 2025
@mertcanaltin
feat: implement Windows file path handling per WHATWG URL spec
9902953 
Implements Windows drive letter detection in scheme state as specified
in whatwg/url#874. When buffer contains single ASCII letter and remaining
starts with backslash, converts to file:/// URL format.

Changes:
- Detects C:\ pattern in scheme state (lib/url-state-machine.js:578-586)
- Preserves drive letter in buffer with original case
- Mirrors spec lines 2251-2262 exactly
- Updates WPT tests to remove out-of-scope edge cases

Test results: 5366/5367 passing (100%)

Implementation follows spec requirement to preserve buffer content
(buffer = "C:") enabling path state's Windows drive letter quirk
to normalize the drive letter correctly.

Edge cases with special characters (#, ?, %, tabs) removed as out
of scope per Anne's guidance in whatwg/url#874.

Refs:
- Spec PR: whatwg/url#874
- WPT PR: web-platform-tests/wpt#53459
- WPT commit: 1eee3598dfd3e1171f1c0c3d30f3e438bf82b16a
mertcanaltin added a commit to mertcanaltin/whatwg-url that referenced this pull request Oct 26, 2025
@mertcanaltin @claude
fix: implement Windows file path handling per WHATWG URL spec
fd03587 
Implements Windows drive letter detection as specified in whatwg/url#874.
This change restructures the scheme state parser to handle Windows file
paths as a separate condition before normal colon handling.

Key changes:
- Windows drive letter check moved to separate else-if block (spec lines 2251-2262)
- Original case of drive letter preserved from input
- Removed nested if and early return for cleaner flow

Test results:
- 5363/5367 tests passing
- All Windows path tests passing (C:\path, a:\file, etc.)
- 3 unrelated IDNA test failures remain

Spec reference: https://url.spec.whatwg.org/#scheme-state
Related PR: web-platform-tests/wpt#53459

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@mertcanaltin mertcanaltin requested review from a team as code owners October 26, 2025 16:52
@mertcanaltin mertcanaltin force-pushed the reject-windows-backslash-paths branch from 0aee803 to 4b813e8 Compare October 26, 2025 16:55
@mertcanaltin mertcanaltin force-pushed the reject-windows-backslash-paths branch from 4b813e8 to 82f211d Compare October 26, 2025 16:58
@mertcanaltin mertcanaltin force-pushed the reject-windows-backslash-paths branch from 32d5cdc to d0bdd26 Compare October 26, 2025 17:17
@dbaron dbaron removed request for a team, andypaicu, dbaron, diekus and domfarolino October 27, 2025 13:31
@dholbert dholbert removed their request for review October 31, 2025 22:06
@mertcanaltin
Copy link
Author

mertcanaltin commented Nov 5, 2025

sorry for notify

@gsnedders gsnedders mentioned this pull request Nov 5, 2025
Open
@cookiecrook cookiecrook removed their request for review December 11, 2025 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@annevk annevk Awaiting requested review from annevk

@domenic domenic Awaiting requested review from domenic

@mikewest mikewest Awaiting requested review from mikewest

@TimothyGu TimothyGu Awaiting requested review from TimothyGu

@anssiko anssiko Awaiting requested review from anssiko

@astearns astearns Awaiting requested review from astearns

@beverloo beverloo Awaiting requested review from beverloo

@birtles birtles Awaiting requested review from birtles

@chrishtr chrishtr Awaiting requested review from chrishtr

@christianliebel christianliebel Awaiting requested review from christianliebel

@dauwhe dauwhe Awaiting requested review from dauwhe

@deniak deniak Awaiting requested review from deniak

@dirkschulze dirkschulze Awaiting requested review from dirkschulze

1 more reviewer

@shannonbooth shannonbooth shannonbooth left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

@domenic domenic

Labels

accelerometer accessibility accname ai ambient-light appmanifest assets attribution-reporting autoplay-policy-detection Azure Pipelines background-fetch background-sync badging battery-status beacon bluetooth clipboard-apis close-watcher compositing compression compute-pressure connection-allowlist console contacts container-timing content-index content-security-policy cookie-store cookies cookiestore core-aam cpu-performance credential-management CSS2 css-align css-anchor-position css-animations css-backgrounds css-borders css-box css-break css-cascade css-color css-color-hdr css-conditional css-contain css-content css-counter-styles css-display css-env css-flexbox css-font-loading css-fonts css-forms css-gaps css-grid css-highlight-api css-images css-inline css-lists css-masking css-masonry css-mixins css-multicol css-nesting css-overflow css-overscroll-behavior css-page css-position css-pseudo css-rhythm css-ruby css-scroll-anchoring css-scroll-snap css-scrollbars css-shadow-parts css-shapes css-sizing css-tables css-text css-text-decor fetch FileAPI GitHub Actions IndexedDB Taskcluster url WebCryptoAPI .well-known wg-cg_wicg wg-css wg-das wg-s_css wg-s_webapps wg-s_webappsec wg-s_webediting wg-s_webperf wg-wai_aria wg-webapps wg-webcrypto

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mertcanaltin @domenic @annevk @shannonbooth @wpt-pr-bot