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.

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

Specify URL encoding for links (header+element) #9764

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Specify URL encoding for links (header+element) #9764

wants to merge 5 commits into from

Conversation

noamr
Copy link
Contributor

@noamr noamr commented Sep 20, 2023 •
edited by pr-preview bot
Loading

  • Use the document encoding for link elements
  • Always use UTF8 for link headers/early hints

Closes #9715

  • At least two implementers are interested (and none opposed):
    • …
    • …
  • Tests are written and can be reviewed and commented upon at:
    • …
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • MDN issue is filed: …

(See WHATWG Working Mode: Changes for more details.)

/links.html ( diff )
/semantics.html ( diff )
/urls-and-fetching.html ( diff )

@noamr
Specify URL encoding for links
ec7151c 
- Use the document encoding for link elements
- Always use UTF8 for link headers/early hints

Closes whatwg#9715
annevk
annevk reviewed Sep 20, 2023
View reviewed changes
source Show resolved Hide resolved
annevk
annevk reviewed Sep 25, 2023
View reviewed changes
source Show resolved Hide resolved
source Show resolved Hide resolved
annevk
annevk reviewed Sep 25, 2023
View reviewed changes
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good modulo some tiny nits. Would like @domenic to sign off.

source Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Show resolved Hide resolved
@noamr
Copy link
Contributor Author

noamr commented Sep 25, 2023

Looks good modulo some tiny nits. Would like @domenic to sign off.

Thanks, fixed nits.
Also WPTs need to cover this, cc @bashi

domenic
domenic approved these changes Sep 28, 2023
View reviewed changes
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with the remaining nit fixed.

source Show resolved Hide resolved
@domenic domenic added needs tests Moving the issue forward requires someone to write tests topic: link topic: resource hints (inc. preload) labels Sep 28, 2023
@domenic
Copy link
Member

domenic commented Oct 23, 2023

@noamr any progress on writing web platform tests for this?

@noamr
Copy link
Contributor Author

noamr commented Oct 23, 2023

@noamr any progress on writing web platform tests for this?

@bashi do you have bandwidth for this? I currently do not.

@bashi
Copy link

bashi commented Oct 26, 2023

Neither do I, unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@annevk annevk annevk left review comments

@domenic domenic domenic approved these changes

Assignees
No one assigned
Labels
needs tests Moving the issue forward requires someone to write tests topic: link topic: resource hints (inc. preload)
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Take encoding into account when parsing link headers & early hints
4 participants
@noamr @domenic @bashi @annevk