fix(html): keep the host's locale out of the css we write by andiwand · Pull Request #828 · opendocument-app/OpenDocument.core

andiwand · 2026-09-06T10:12:32Z

🤖 Generated with Claude Code

Independent of #826 — nothing here needs C++23, so it can land first.

The bug

html::color built its rgba(…) through an un-imbued std::stringstream, so
the alpha picked up whatever global locale the host had set. Reproduced
against main:

  default locale : rgba(0,0,0,0.501961)
  de_DE locale   : rgba(0,0,0,0,501961)     ← four commas; not a colour

The declaration is dropped by the browser, so every semi-transparent fill,
background, border and stroke silently loses its colour. html::color is called
from every style writer.

util::number::to_string_significant already knew about this and imbued the
classic locale by hand — svg/AGENTS.md even documents why svg numbers must
go through it. Three other places never got the memo:
util::string::to_string(double, int), html::human_size and
pdf::Object::to_stream.

There is a regression test: html_common.a_global_locale_does_not_reach_the_css.
It fails on main and passes here, and skips where de_DE.UTF-8 is not
installed.

Why fmt and not `std::format`

std::format is locale-independent too and is in the standard we are moving to,
but it is unusable in this project on any slice. libc++ reaches it through a
floating-point std::to_chars marked introduced in macOS 13.3 / iOS 16.3, and
apple.jinja deploys to macOS 12 / iOS 15. <format> instantiates that
formatter whatever the argument types are, so even
std::format("{}:{}", host, port) fails to compile, and one call anywhere in
src/ breaks the framework build. (_LIBCPP_DISABLE_AVAILABILITY makes it
compile and is a trap: the dylib then carries 9 undefined float to_chars
symbols even when no double is formatted, which is a dyld failure at load on
macOS 12 rather than a compile error.)

fmt carries its own float formatting, so none of that applies. Verified to build
to an object file on apple-clang at macOS 12 and iOS 15 deployment
targets, Android NDK 28.1, emsdk 3.1.73 and libstdc++, with zero undefined
to_chars in a dylib built at macOS 12.

Output is unchanged, deliberately

The reference html is byte for byte what it was — {:.{}f} is std::fixed +
setprecision, {:.4g} is bare setprecision(4), {:g} is a default-formatted
double and {:06x} is setw(6) + setfill('0') + hex. Each spelling was
diffed against a stream over 1.2M values, and against std::format over the
same, before being used: 0 mismatches.

odr_test: 1443 passed, 6 skipped, 0 failed (1449 total, +1 new).

This is not a perf change

I checked, because it looked like one. to_string_significant is ~2.5× faster
through fmt in isolation (195.8 → 77.1 ms per million), but a real render calls
it 825 to 20,444 times, not millions — so it is worth 0.1–2.4 ms on a 37 ms
document, which is noise. Timing translate over a drawing-heavy .svm shows
no change.

So the hot html element and style writers keep their
"…" + std::to_string(n) + "…" concatenations; fmt is not faster than those at
these string lengths (18.6 vs 18.3 ms/1M). Only the four sites that were
building numbers through a stream move — those were 5× slower than either, and
were the ones carrying the bug.

Compile time is a wash to slightly better: <fmt/format.h> against the compiled
package costs less per TU than the <sstream> + <iomanip> it replaces
(198 ms vs 232 ms). The header-only mode is the expensive one and is not used.

Changes

conanfile.py + conan.lock + CMakeLists.txt — fmt/11.2.0, linked
PRIVATE, so nothing reaches the public headers.
util/number_util.cpp — drops the imbue dance; the guarantee is now the
formatter's.
util/string_util.cpp, html/common.cpp, html/filesystem.cpp,
pdf/pdf_object.cpp — the four un-imbued sites. pdf_object also stops
leaving setprecision(4) on a stream it shares.
AGENTS.md — the convention and the reason std::format is not it.

`html::color` built its `rgba(…)` through an un-imbued `std::stringstream`, so the alpha picked up whatever global locale the host had set. Under a german one a translucent colour came out as `rgba(0,0,0,0,501961)` — four commas, not a colour, and the declaration is dropped. `util::number::to_string_significant` already knew this and imbued the classic locale by hand; three other places did not. All of them format through `fmt` now, which ignores the global locale unless asked. `std::format` would have done as well, but is unusable here: libc++ reaches it through a floating-point `std::to_chars` marked unavailable before macOS 13.3 / iOS 16.3, and the apple profiles deploy to macOS 12 / iOS 15, so one call anywhere in `src/` fails the framework build whatever the argument types are. Output is unchanged everywhere else, and deliberately so — the reference html is byte for byte what it was. `{:.{}f}` is `std::fixed` + `setprecision`, `{:.4g}` is bare `setprecision(4)`, `{:g}` is a default-formatted double and `{:06x}` is `setw(6)` + `setfill('0')` + `hex`; each was checked against a stream over 1.2M values before being used. `pdf_object` stops leaving `setprecision(4)` on a stream it shares while it is at it. The hot html element and style writers keep their `"…" + std::to_string(n)` concatenations — `fmt` is no faster there, and this is not a perf change: a render calls `to_string_significant` a few thousand times, not a few million. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoTh7BEUSL2z9BBThgsEi7

The whole matrix already supports it: clang 18, gcc 14, apple-clang, MSVC 19.40, NDK 28.1 and emsdk 3.1.73. It costs no source change at all — the two transitive includes libc++'s C++23 headers stopped handing out went in with #827, which is the whole of it. Three ceilings sit under the standard, and AGENTS.md records all three because none is discoverable from a local build. The library half is capped by emsdk 3.1.73's libc++ 18.1, the oldest here and the newest emsdk conan-center packages. `std::format` is unusable on any slice — the apple profiles deploy to macOS 12 / iOS 15, and libc++ marks the floating-point `to_chars` that `<format>` instantiates as macOS 13.3 / iOS 16.3, which is why #828 formats through `fmt`. And NDK 28.1's clang 19 segfaults on a capturing recursive lambda taking `this auto self`. The public headers stay C++20 — no `target_compile_features(odr PUBLIC …)` and no `cppstd` in `package_info`, so a consumer picks its own standard, and `check_min_cppstd` sits in `validate_build` where it constrains building `odr` rather than using it. On MSVC there is no `/std:c++23`; CMake maps `CXX_STANDARD 23` to `/std:c++latest`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QoTh7BEUSL2z9BBThgsEi7

* build!: raise the standard to C++23 The whole matrix already supports it: clang 18, gcc 14, apple-clang, MSVC 19.40, NDK 28.1 and emsdk 3.1.73. It costs no source change at all — the two transitive includes libc++'s C++23 headers stopped handing out went in with #827, which is the whole of it. Three ceilings sit under the standard, and AGENTS.md records all three because none is discoverable from a local build. The library half is capped by emsdk 3.1.73's libc++ 18.1, the oldest here and the newest emsdk conan-center packages. `std::format` is unusable on any slice — the apple profiles deploy to macOS 12 / iOS 15, and libc++ marks the floating-point `to_chars` that `<format>` instantiates as macOS 13.3 / iOS 16.3, which is why #828 formats through `fmt`. And NDK 28.1's clang 19 segfaults on a capturing recursive lambda taking `this auto self`. The public headers stay C++20 — no `target_compile_features(odr PUBLIC …)` and no `cppstd` in `package_info`, so a consumer picks its own standard, and `check_min_cppstd` sits in `validate_build` where it constrains building `odr` rather than using it. On MSVC there is no `/std:c++23`; CMake maps `CXX_STANDARD 23` to `/std:c++latest`. * refactor: let deducing `this` write the const overload Forty-two accessors were the same body twice, once for each constness, with only the spelled-out return type telling the two apart. An explicit object parameter deduces that, so each pair is one function returning `T &` or `const T &` from a single body. The shared registry (#823) is where it pays most: `SideTable` and `SortedSideTable` had written the workaround out by hand — two public overloads delegating to a `static` helper templated on the object, under a comment naming the trick — and every engine then repeated the pair for each of its payload accessors. Both go, and a registry's accessor is [[nodiscard]] auto &text_element_at(this auto &self, const ElementIdentifier id) { return self.m_texts.at(id); } `pdf`'s `Array` and `Dictionary` lose eleven more. `ooxml_text_list`'s numbering walk would have dropped its Y-combinator with them, but NDK 28.1's clang 19 segfaults on a capturing lambda that recurses through an explicit object parameter, so it keeps passing itself along and says why. * perf: stop zeroing the buffer the next read overwrites anyway `resize` fills the new tail with zeros; every one of these then writes over all of it. `resize_and_overwrite` hands the chunk over unwritten instead — 12.3 GB/s to 16.7 GB/s on `read_u8s` against an in-memory stream, which is what a zip entry is. The callback must not throw, so a short read shrinks the string back to the offset it started from and the throw happens at the call site. `ppt`'s `read_raw_text_bytes` loses its second `resize` with it: the callback returns `gcount()` and the string is already the right length. `xls_io`'s string body is left alone — its `read_bytes` throws from inside, and a non-throwing path just for this is not worth the buffer it saves. * refactor: build the two derived vectors with `ranges::to` Both loops did nothing but map a range onto a vector, and both fed it straight into one call. `views::transform | ranges::to<std::vector<std::string>>()` says that in the expression that uses it, so neither needs a named variable any more. `type1_charstring` takes the iterator-pair `std::reverse` next to it with them, per the ranges convention. * test(build): hold the public headers to C++20 The bump made `odr` C++23 and deliberately did not pass that on — no `target_compile_features(odr PUBLIC …)`, no `cppstd` in the conan `package_info` — so a consumer keeps whatever standard it picked, as long as `src/odr/*.hpp` stays C++20. Nothing checked that, and a `this auto &self` in a public header would have broken someone else's build rather than ours. `odr_public_headers_cpp20` includes all seventeen of them and compiles at C++20. One object file, no test data, no link, no gtest — it either compiles or it does not. It lives in `test/CMakeLists.txt` and so builds under `ODR_TEST`, which is also what keeps it out of the conan package: `exports_sources` ships no `test/`, and an unconditional target naming a file that is not there fails the package build at generate time. * docs: cut the prose from the comments this branch adds The C++23-does-not-propagate rationale now lives in AGENTS.md alone; the test file and its cmake target state what they are and stop there. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

andiwand mentioned this pull request Sep 6, 2026

build!: raise the standard to C++23, and spend it #826

Merged

andiwand force-pushed the feat/fmt branch from de937d5 to a981c90 Compare September 6, 2026 10:19

andiwand merged commit 2cc1ace into main Sep 6, 2026
22 checks passed

andiwand deleted the feat/fmt branch September 6, 2026 10:20

Navigation Menu

Uh oh!

Uh oh!

fix(html): keep the host's locale out of the css we write - #828

fix(html): keep the host's locale out of the css we write#828
andiwand merged 1 commit into
mainfrom
feat/fmt

andiwand commented Sep 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

andiwand commented Sep 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The bug

Why fmt and not std::format

Output is unchanged, deliberately

This is not a perf change

Changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

andiwand commented Sep 6, 2026 •

edited

Loading

Why fmt and not `std::format`