Normally I am a strong proponent of adjusting implementations one small step at a time, but this is a bit of an extreme case.
The current state of UrlCanonicalize() is a prototypical example of "spaghetti code": the implementation has been repeatedly adjusted to fix a new discovered edge case, without properly testing the scope of the new logic, with the effect that the current logic is a scattered mess of conditions that interact in unpredictable ways.
To be fair, the actual function is much more complicated than one would anticipate, and the number of new weird edge cases I found while trying to flesh out existing logic was rather exhausting.
I initially tried to "fix" the existing implementation one step at a time. After racking up dozens of commits without an end in sight, though, and having to waste time carefully unravelling the broken code in the right order so as to avoid temporary failures, I decided instead to just fix everything at once, effectively rewriting the function from scratch, and this proved to be much more productive.
Hopefully the huge swath of new tests is enough to prove that this new implementation really is correct, and has no more spaghetti than what Microsoft made necessary.
-- v2: shlwapi/tests: Add many more tests for UrlCanonicalize(). kernelbase: Use scheme_is_opaque() in UrlIs(). kernelbase: Reimplement UrlCanonicalize(). kernelbase: Do not canonicalize the relative part in UrlCombine(). shlwapi/tests: Move UrlCombine() error tests out of the loop. shlwapi/tests: Use winetest_push_context() in test_UrlCanonicalizeA(). shlwapi/tests: Remove the unused "wszExpectUrl" variable from check_url_canonicalize(). shlwapi/tests: Move the UrlCanonicalize() tests into test_UrlCanonicalizeA(). urlmon/tests: Add basic tests for CoInternetParseUrl(PARSE_CANONICALIZE).