Skip to content

Record the original for every rewritten URL attribute - #338

Draft
epheterson wants to merge 1 commit into
openzim:mainfrom
epheterson:generalise-orig-attrs
Draft

epheterson wants to merge 1 commit into
openzim:mainfrom
epheterson:generalise-orig-attrs

Conversation

@epheterson

Copy link
Copy Markdown
Contributor

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.

Widens the script-src case to src, href and xlink:href on any tag.

wombat's getAttribute override looks for the recorded original first and
otherwise calls extractOriginalURL, which can only reverse rewrites
wombat itself performed. A value rewritten here is unrecognisable to it,
so it prepends the original scheme to what is already a relative path.

That is openzim/warc2zim#413 as well as #473: same cause, two symptoms.

Needs the matching wombat change, since retrieveWBOSRC is SCRIPT-only
until then and nothing reads the other attributes back.

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

poster is deliberately excluded: wombat rewrites it, this rewriter does
not, so it would record nothing.
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7908b7b) to head (94b65ce).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #338   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           42        42           
  Lines         2694      2697    +3     
  Branches       398       397    -1     
=========================================
+ Hits          2694      2697    +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

Copy link
Copy Markdown
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not rewrite links twice, once statically and once dynamically

2 participants