wpt icon indicating copy to clipboard operation
wpt copied to clipboard

URL: Update IdnaTestV2 to UTS46 16.0.0

Open rmisev opened this issue 1 year ago • 3 comments

Updates IdnaTestV2-parser.py for version 16.0.0 of Unicode IDNA Compatibility Processing [UTS46] and fixes other issues:

  • Adds the encoding="utf-8" parameter to file open calls to run correctly if the default system encoding is not utf-8
  • Fixes the parameter that is written to the output file: --exclude_bidi changed to --exclude-bidi

rmisev avatar Sep 21 '24 16:09 rmisev

Thank you for working on this, but I'm not sure this can be merged as-is.

This results in failures for both Firefox and Safari, both thought to have compliant implementations. I'm not sure I fully trust the updated tests. Especially given that we still have some open issues against the URL Standard about sorting out some IDNA issues with UTS 46.

Did you investigate the failures?

annevk avatar Sep 23 '24 13:09 annevk

I think it's normal that about 70 tests fail with Firefox and Safari. About the same number of tests failed when I tested with my UTS46 15.1.0 implementation. There are no failed tests on my UTS46 16.0.0 implementation. I think there may be similar results with browsers when they switch to UTS46 16.0.0

rmisev avatar Sep 23 '24 18:09 rmisev

That's fair, I guess the problem is that I'm not entirely convinced that the UTS 46 update is fault-free given the response we've been given to some of the issues we've raised.

annevk avatar Sep 23 '24 18:09 annevk

This is what I'd like to do to get more clarity on the changes here (help welcome, but I suspect I can get to it):

  • Create a list of all inputs whose output changed.
  • Create a list of all removed inputs. (If there are any I think I would consider this an error and we should put those in a separate test.)
  • Create a list of all new inputs and outputs. (And then ideally cross-reference those against browser results.)

Also given Interop 2024 we won't be able to land this until somewhere early next year, probably February.

annevk avatar Nov 28 '24 10:11 annevk

FWIW, I have not tested this PR, but I have tested the upstream IdnaTestV2.txt in combination of IDNA back end that Firefox uses (but testing in isolation of Firefox) and ICU4X trunk, which has Unicode 16.0 data, and the upstream test passes that way.

(I had to change the expectation for the root label dot handling, but since the URL Standard does not expose verifyDnsLength to via a browser API, that difference shouldn't be visible via WPT.)

hsivonen avatar Nov 28 '24 17:11 hsivonen

I wrote the comparison script: https://gist.github.com/annevk/189bcb339d0f5dffbd79b44bacfa7e33

There's 66 tests removed here. That seems problematic. We should keep them. Probably in a separate file. But I'm not sure how we can verify that their results are still accurate. Perhaps we can assume Henri's implementation is correct for now?

There's one input that now fails to parse whereas it used to succeed: xn--xn--a--gua.pt. That's discussed in https://github.com/whatwg/url/issues/803 and seems like an improvement, although notably no browser fails on this today.

There's also lots of added tests. I guess we can assume those are correct.

Then separately we need to figure out if we can enable more tests now, e.g., BiDi. Though it wasn't quite clear if those issues were resolved now. That's probably best tracked separately though as this issue already covers a lot.

annevk avatar Nov 28 '24 17:11 annevk

Then separately we need to figure out if we can enable more tests now, e.g., BiDi. Though it wasn't quite clear if those issues were resolved now.

According to the UTC #179 PAG report (see: "6.1 Proposals to modify CheckBidi handling (UTS 46)"), Bidi changes are planned for Unicode 17.0.

rmisev avatar Nov 28 '24 21:11 rmisev

I think the changes made here make it so that encodeHostEndingCodePoints() in IdnaTestV2.window.js is no longer needed. We should instead assert/filter :, /, ?, #, and \ in the parser.

annevk avatar Nov 29 '24 16:11 annevk

I created #49443 as a sibling PR to cover the removed tests (whose input does not contain ?). Review appreciated. Again, given Interop 2024 we'll have to wait a bit with landing this.

annevk avatar Nov 29 '24 17:11 annevk

@rmisev I think we can make changes to url/ again. Are you interested in addressing the above feedback?

annevk avatar Mar 03 '25 11:03 annevk

I think the changes made here make it so that encodeHostEndingCodePoints() in IdnaTestV2.window.js is no longer needed. We should instead assert/filter :, /, ?, #, and \ in the parser.

I agree. I've tried filtering these characters and I see that it doesn't change the parser output, probably because there are no tests with these characters (except ?). But we cannot guarantee that such tests will not appear in the future. I will make changes soon.

rmisev avatar Mar 03 '25 20:03 rmisev