Refactor query state to operate on a buffer #558
Conversation
|
Thanks, it's incomplete still as the percent-encoding algorithm will also need changes. I do think using a buffer coupled with whatwg/encoding#235 (comment) gives us the "nicest" way of making this work, but happy to receive feedback otherwise. (The one weirdness is that we don't have a per-code-point percent-encode with arbitrary encoding anymore, but no caller really needs that. The UTF-8 caller can just pass its single code point as a string.) |
|
The corresponding Encoding PR for the second commit in this PR hasn't landed yet and as such it'll fail Build, but it's still worth reviewing at this point I think, especially for non-editorial concerns. |
|
I plan to rebase this Monday and make it pass Build now that whatwg/encoding#238 has landed. |
There was a problem hiding this comment.
It looks like I can't really test this with jsdom/whatwg-url since we're still UTF-8 only. So my review of the second commit is just editorial.
Maybe I should sink some time into fixing that, but doing it properly would probably require a from-scratch implementation of the Encoding standard which exposes all of the spec-level primitives. Probably not something I can tackle any time soon...
|
Can we get web platform tests for this that are URL-specific? Probably not part of the JSON files though. |
annevk
force-pushed
the
annevk/iso-2022-jp
branch
from
18ae07a
to
22e8bd3
Compare
annevk
force-pushed
the
annevk/iso-2022-jp
branch
from
22e8bd3
to
891f55c
Compare
|
Thanks for the feedback! I put up some fairly basic tests over at web-platform-tests/wpt#26317 but they should be easy to expand upon if we all think that format is workable. |
Since the ISO-2022-JP encoder is stateful, percent-encoding needs to hold onto an instance of the encoder and manually perform error handling. This also requires the input to be the full string rather than individual code points as otherwise the callers of percent-encoding would need to be aware of this too. (As UTF-8 encoding cannot fail this problem does not affect those endpoints.) Builds on this Encoding PR: whatwg/encoding#238. Tests: web-platform-tests/wpt#26158 and web-platform-tests/wpt#26317. Fixes #557.
annevk
force-pushed
the
annevk/iso-2022-jp
branch
from
891f55c
to
38f6ae8
Compare
Follows whatwg/url#558. This also updates the documentation to make it clear that we have no encoding handling, and removes the mention of encodingOverride (which was ignored by the code).
Follows whatwg/url#558. This also updates the documentation to make it clear that we have no encoding handling, and removes the mention of encodingOverride (which was ignored by the code).
After whatwg/url#558, the form submission algorithm is the only place in the web platform that uses "encode". However, although for application/x-www-form-urlencoded and text/plain "encode" is clearly linked, multipart/form-data instead describes an equivalent algorithm. This change fixes that.
After whatwg/url#558, the form submission algorithm is the only place in the web platform that uses "encode". However, although for application/x-www-form-urlencoded and text/plain "encode" is clearly linked, multipart/form-data instead describes an equivalent algorithm. This change fixes that.
(See WHATWG Working Mode: Changes for more details.)
Preview | Diff