[benoit74]

Summary

• ArticleUrlRewriter resolved relative links (and base_href) with urllib.parse.urljoin, which collapses empty path segments while merging a relative reference onto the base URL. When a page's own URL contains //, this silently collapsed it before normalize() could see it, so the rewritten link pointed to an entry that doesn't exist in the ZIM.
• Replaced it with a custom join following RFC 3986 section 5.3 (merge + remove-dot-segments) that keeps empty segments, matching how browsers resolve such references.
• Checked the JS (wombat) side: it already resolves relative URLs correctly via uri-js's URI.resolve, which doesn't have this deviation, so no fix was needed there — added a regression test to document/lock in that behavior instead.

Fix #346

[benoit74]

@Sinkleberg / @anshuman83-40 I would be happy to have your feedback on this one. Cannot ask a formal review but would be happy to have an informal one ^^

Looks like there is no easy way besides re-implementing Python function ...

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (2a661fe) to head (a576f69).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #358   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         2817      2852   +35     
  Branches       416       428   +12     
=========================================
+ Hits          2817      2852   +35     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[anshuman83-40]

Thanks for picking this up! I had a look and it seems right to me:

• _urljoin gives the expected result for all 42 examples in RFC 3986 §5.4 (normal and abnormal), and for the // cases (other.html, ../z.html, ?page=2, #top, //cdn…) from a page like https://example.com/x//y/page.html.
• On 200k random base/reference pairs without // it returns exactly what urllib.parse.urljoin returns, so behaviour shouldn't change elsewhere.

About the coverage: in _remove_dot_segments, the ../, ./ and bare ./.. branches can't be reached since the merged path always starts with / (as the docstring says), so they could be dropped or marked # pragma: no cover. Same for if not base and the non-uses_netloc case. The one reachable gap is a query-only reference like ?page=2 (the if not query branch), which might be worth a test.

Small thing: _resolve_relative_reference in tests/rewriting/test_url_rewriting.py (from #344) now duplicates _remove_dot_segments, so the round-trip test could probably reuse it.

[benoit74]

Thanks for the thorough check! Pushed 5fdf31f addressing the coverage points:

• Dropped the unreachable branches in _remove_dot_segments (../, ./, bare ./..) and in _urljoin (if not base, the uses_netloc guard — uses_relative schemes are always a subset of uses_netloc in cpython, so that check could never be false once we get past the earlier scheme not in uses_relative return).
• Added a test for the query-only reference case (?page=2), which was the one reachable gap.
• _resolve_relative_reference in the tests now reuses _remove_dot_segments instead of duplicating it.

Branch coverage on _urljoin/_remove_dot_segments is now 100% with no partials.