[epheterson]

Follow-up to #337, widening it from <script src> to src, href and xlink:href on any tag, as you suggested.

• Also fixes Do not rewrite links twice, once statically and once dynamically warc2zim#413: same cause. extractOriginalURL prepends the original scheme when it cannot recognise a value as one wombat rewrote, so an href rewritten here comes back as https:// followed by a relative path.
• Needs Record any URL attribute's original, not just a script src webrecorder/wombat#238 first. Until that lands, retrieveWBOSRC is SCRIPT-only and nothing reads the other attributes back.
• Size on en.wikipedia.org/wiki/Whale (1,766 links): 152 attributes added, 0.9% larger gzipped. Most links are fragments or already relative and record nothing.
• 18 existing expectations change, because fixtures with rewritten links now carry their originals.
• poster is excluded: wombat rewrites it, this rewriter does not, so it would record nothing.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (6613ac5) to head (966e301).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #338   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         2798      2801    +3     
  Branches       417       416    -1     
=========================================
+ Hits          2798      2801    +3     

☔ 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.

[benoit74]

Thank you very much!

Just for clarification: this change concerns only ZIMs built with scraperlib rewriting, i.e. zimit/warc2zim and mindtouch ATM (the fact you tested on a WPEN page could lead someone to be concerned it might impact mwoffliner ZIMs, which is not the case - at this stage of mwoffliner development at least).

Putting PR in draft until wombat PR settles, so we do not merge this by accident (even if it would be harmless, but not really useful either).

[benoit74]

Now that upstream PR has been merged, @epheterson do you confirm that we can move this forward and merge?

[ikreymer]

Can someone explain why the change in wombat (and the follow-up) is needed for zim replay, but not for our existing server-side and service worker-based replay? As I recall, the original workaround was a hack having to do with being able to add the modifier like js_ / esm_ that is particular to scripts, and needing to resolve relative paths to absolute for that reason, though there could be a better solution. Why is it that you're running into a double rewriting issue?

[benoit74]

As I recall, the original workaround was a hack having to do with being able to add the modifier like js_ / esm_ that is particular to scripts, and needing to resolve relative paths to absolute for that reason, though there could be a better solution.

I do not recall about this hack, and looking at our codebase I do not see matching code.

Why is it that you're running into a double rewriting issue?

Maybe we just miss the hack (because we ignored it or forgot about it) and went into a too complex solution.

ATM we have no way to know JS-side that a URL has already been rewritten. Which causes issue when for instance JS code is fetching a src attribute to make the web request "on its own": the src attribute was already rewritten, but JS making the request "on its own" it passes an already rewritten src to wombat which rewrites it again.

For instance on noted.lol, there is a script which fetches some rewritten src, and uses them to add a click event listener + remove event propagation, so that any click on a link is handled by JS.

[epheterson]

Yup, good to go! Bumped the pin and updated the changelog.

[epheterson]

The difference is that a ZIM has no replay prefix. URLs are rewritten at scrape time to plain relative paths inside the archive, like ../github.com/openzim/zimit%3Fref%3Dnoted.lol, so a rewritten path looks just like an original relative one, and there's nothing for extractOriginalURL to strip.

That rewrite also loses information: whether the original was relative or absolute, and the ? gets folded into the path. So when page JS calls getAttribute, wombat can't work back to what the server sent. Some JS needs exactly that string (Turbopack matches chunk names on it), so the scraper records it.

[benoit74]

LGTM, thank you.

@ikreymer I will merge this because I feel like it is providing expected value, but I'm still interested to continue the discussion you've started (thank you for speaking up btw), always good to have a good shared understanding + identify potential better solutions.