fix: protect page count alias from UBA reordering (issue #1925) - #1929
fix: protect page count alias from UBA reordering (issue #1925)#1929prateek-dagar wants to merge 1 commit into
Conversation
b3a99d5 to
919c52e
Compare
|
Hi @andersonhc, I updated the branch to fix the markdown formatting issue:
All tests are now passing! |
andersonhc
left a comment
There was a problem hiding this comment.
I wonder if we could encapsulate the placeholder handling inside BidiParagraph.
Perhaps BidiParagraph could accept an optional alias or protected_token parameter.
When present, it would:
- Select an unusual sentinel character that does not occur in the input.
- Replace every occurrence of the alias with that sentinel.
- Run the bidi algorithm.
- Replace the sentinel with the original alias in each string returned by
get_bidi_fragments().
This would keep the sentinel internal to the bidi implementation. The caller would receive {nb} again, allowing the existing _preload_font_styles() logic to create TotalPagesSubstitutionFragment normally. It would also prevent the sentinel from reaching font lookup and fallback handling.
The sentinel should specifically have bidi class EN (“European Number”), rather than S or BN. The eventual page count consists of ASCII digits, which are also EN. Matching that class is important because the Unicode bidi algorithm resolves adjacent punctuation using the number’s bidi class. For example, 10/{nb}, {nb}-10, and {nb}% should be resolved as numeric LTR runs inside an RTL paragraph. A non-numeric sentinel can cause /, -, or % to be assigned to a different directional run.
Could we create a small list of unusual EN characters, selecting the first one absent from the text?
The available characters can be reviewed here: https://www.compart.com/en/unicode/bidiclass/EN.
U+1D7FF is good candidate for main sentinel in my opinion, and we could add a list of fallback ones in BidiParagraph
It would be good to add test cases involving punctuation, for example:
"אבג 10/{nb} דהו"
"אבג {nb}-10 דהו"
"אבג ({nb}%) דהו"
On the tests make 2 calls to cell(), one with the alias and one with numeric values, and both must resolve identically on the output pdf.
|
Thanks @andersonhc! |
919c52e to
4bcba20
Compare
|
Hi @andersonhc ,
|
Changes
Replace the alias with a single placeholder PUA character (
\uE000with collision-safe scanning) before UBA bidi processing.Because a single character is atomic, it survives UBA reordering intact at its correct semantic location.
Reconstruct the bidi fragments by expanding the PUA placeholder back into a
TotalPagesSubstitutionFragment.Fixes #1925
A unit test is covering the code added / modified by this PR
In case of a new feature, docstrings have been added, with also some documentation in the
docs/folderA mention of the change is present in
CHANGELOG.mdThis PR is ready to be merged
By submitting this pull request, I confirm that my contribution is made under the terms of the GNU LGPL 3.0 license.