Skip to content

fix: protect page count alias from UBA reordering (issue #1925) - #1929

Open
prateek-dagar wants to merge 1 commit into
py-pdf:masterfrom
prateek-dagar:fix-issue-1925
Open

fix: protect page count alias from UBA reordering (issue #1925)#1929
prateek-dagar wants to merge 1 commit into
py-pdf:masterfrom
prateek-dagar:fix-issue-1925

Conversation

@prateek-dagar

Copy link
Copy Markdown

Changes
Replace the alias with a single placeholder PUA character (\uE000 with 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/ folder

  • A mention of the change is present in CHANGELOG.md

  • This 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.

Comment thread test/test_alias.py
@prateek-dagar
prateek-dagar force-pushed the fix-issue-1925 branch 2 times, most recently from b3a99d5 to 919c52e Compare August 28, 2026 15:10
@prateek-dagar

Copy link
Copy Markdown
Author

Hi @andersonhc,

I updated the branch to fix the markdown formatting issue:

  • Changed placeholder to \u001f (neutral character) to prevent UBA from splitting RTL runs and breaking markdown tags.
  • Overrode page count fragment direction to LTR to prevent numbers from being reversed to "21" in RTL text.
  • Added test_alias_in_rtl_text_rtl_alias to verify custom RTL script aliases shape correctly.

All tests are now passing!

@andersonhc andersonhc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@prateek-dagar

Copy link
Copy Markdown
Author

Thanks @andersonhc!
I'll encapsulate the alias substitution inside BidiParagraph using U+1D7FF (and fallback EN characters) so that the sentinel stays completely internal to bidi processing and resolves punctuation rules correctly. I'll also add the comparison tests for 10/{nb}, {nb}-10, and ({nb}%)

@prateek-dagar

Copy link
Copy Markdown
Author

Hi @andersonhc ,
I have incorporated these changes, please have a look at it.

  1. Sentinel Placeholder in BidiParagraph:

    • BidiParagraph now substitutes the alias with an EN (European Number) sentinel digit before executing the Unicode Bidirectional Algorithm (UBA), ensuring page placeholders surrounded by RTL text, punctuation (slashes, hyphens, parentheses), and percentage signs are treated with numeric directionality.
  2. Markdown Styling across Directional Runs:

    • Updated _preload_bidirectional_text() to track and propagate markdown emphasis states (bold, italics, strikethrough, underline) across directional segments, allowing continuous styling (e.g. --אבג {nb} דהו--) across RTL/LTR transitions.
  3. Test Suite & Side-by-Side Verification:

    • Added test cases in test/test_alias.py covering standard aliases, custom aliases, RTL script aliases, adjacent punctuation, and markdown formatting.
    • Added parameterized side-by-side comparison tests comparing the alias output directly against literal number renderings in the same document.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support page number alias substitution with bidi / RTL text shaping

2 participants