Repository navigation
splitNode slices xml_str with positions read in XML.jl's normalized copy #472
Description
Hi @TimG1964, since #471, XLSX master fails to precompile against the main branch of XML.jl:
ArgumentError: XML tokenizer error at position 835: expected attribute name or tag close
XML.jl's Downstream job tests XLSX master against each XML.jl pull request. It has failed since #471 was merged (run).
Cause. splitNode takes start and stop from _element_span(target_lazy), which reads them in target_lazy.data. It then slices xml_str with them. With XML.jl 0.4.6, target_lazy.data is xml_str itself. XML.jl main normalizes line ends on read, per XML 1.0 §2.11 (JuliaData/XML.jl#133, not yet released). When the input contains a CR LF, the readers retain a normalized copy, one byte shorter per CR LF.
The worksheet in src/data/blank.xlsx has one CR LF, after the XML declaration. The cut therefore starts one byte early, on the > that closes <sheetFormatPr …/>:
… x14ac:dyDescent="0.2"/<sheetData/>><pageMargins …
Before #471, XML.splicetext(target_lazy, replacement) spliced target_lazy.data, so the positions and the string agreed.
Fix. Slice the string the positions come from:
src = target_lazy.data
stripped_xml = string(SubString(src, firstindex(src), prevind(src, start)),
replacement, SubString(src, stop))With this change, XLSX's test suite passes against XML.jl main, and the stub still reads back with XML.jl 0.4.6. sst_load! already slices sst_root.data, the string its cursor reads, so it needs no change.
XLSX's own CI resolves XML.jl 0.4.6, which does not normalize line ends, so it stays green.
Fixing this in XLSX and releasing the fix seems simpler to me than splitting the 0.4.7 changes now on XML.jl main. Such a split would mean releasing 0.4.7 without JuliaData/XML.jl#133 and the changes built on it since.
Activity
TimG1964 commentedon Oct 5, 2026
Thanks, @mathieu17g. I'll try to do this today.
The reason for this reversion was that the public replacement functions created a noticeable performance regression (XML.sourcespan/splicetext currently run the full tokenizer over the element, about 6x the raw scanner's cost on a large file, re: JuliaData/XML.jl#157).
I'll share a few benchmarks with this proposed change when I get them done. I hope we can avoid the speed regression this time!
Thanks,
Tim
mathieu17g commentedon Oct 5, 2026
Hello, the proposed fix should not cause any performance regression with the upcoming v0.4.7, since you will just use the normalized source. Normalizing the source won't be optional in v0.4.7: XML 1.0 §2.11 requires it for conformance.
With v0.4.6, there won't be any regression either, since there is no prior normalization of CR LF into LF.
TimG1964 commentedon Oct 5, 2026
Thanks @mathieu17g.
Fixed in #PR as you suggested:
splitNodenow slicestarget_lazy.data, the string the span offsets point into, in the same waysst_load!slicessst_root.data. No other code in XLSX mixes token offsets with a different string.Verification
splitNodetests onCRLFworksheets, with and without attributes onsheetData. They compare the output with line endings normalised, so they pass on both kinds of XML.jl. Under XML.jl main the old code gives…/><sheetData/>ata>…, which still parses, so the tests compare the whole output rather than just checking it parses.One side effect: with a line-ending-normalising XML.jl, the stripped worksheet comes out with
LFline endings instead ofCRLF. XML 1.0 §2.11 treats the two the same.Benchmarks
(bench/, XML 0.4.6, Julia 1.13.1, 103 fixture/benchmark pairs, median times)
No measurable change. Allocations and memory differ only by the same few allocations seen between two runs of the same code. Timing differences go both ways and are within run-to-run noise; the largest swings are on sub-millisecond small/* and single_cell cases. This is expected: on XML 0.4.6 target_lazy.data === xml_str, so the fixed code does exactly the same work.
Fix JuliaData#472 (failure to compile with XML.jl) (JuliaData#473)