Skip to content

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

Open
prateek-dagar wants to merge 2 commits 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 2 commits 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
pdf.set_text_shaping(False)
pdf.cell(text="אבג {nb} דהו", align="R")
assert_pdf_equal(pdf, HERE / "alias_in_rtl_text.pdf", tmp_path)

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.

Suggested change
def test_alias_in_middle_with_shaping_markdown(tmp_path):
pdf = fpdf.FPDF()
pdf.add_font("Quicksand", style="", fname=HERE / "fonts" / "Quicksand-Regular.otf")
pdf.add_font("Quicksand", style="B", fname=HERE / "fonts" / "Quicksand-Bold.otf")
pdf.add_page()
pdf.set_font("Quicksand", size=24)
pdf.set_text_shaping(True)
pdf.multi_cell(w=pdf.epw, text="**Pages {nb}** with shaping", markdown=True)
assert_pdf_equal(pdf, HERE / "alias_in_middle_with_shaping_markdown.pdf", tmp_path)

Can you add a test to make sure it's not breaking markdown?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The test case test_alias_in_middle_with_shaping_markdown already exists in the test suite.

Additionally, I have added a new test case test_alias_in_rtl_text_markdown to cover markdown formatting with RTL text shaping.

@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!

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