Skip to content

Narrow include_rtl? to RTL letter scripts only (v0.1.8) - #2

Open
wahidfarid wants to merge 4 commits into
tablecheck:line-wrap-supportfrom
wahidfarid:rope-3815-strip-bidi-formatting
Open

Narrow include_rtl? to RTL letter scripts only (v0.1.8)#2
wahidfarid wants to merge 4 commits into
tablecheck:line-wrap-supportfrom
wahidfarid:rope-3815-strip-bidi-formatting

Conversation

@wahidfarid

@wahidfarid wahidfarid commented Apr 24, 2026

Copy link
Copy Markdown

Problem

Stray invisible BiDi formatting controls — BOM (U+FEFF), RLM (U+200F),
embedding/override (U+202A–U+202E), and isolates (U+2066–U+2069) — pasted
from Excel, iOS keyboards, or RTL input methods into memo fields caused the RTL
reorder path to fire on otherwise-LTR text. The result was scrambled visual
output in generated PDFs (e.g. ! pulled to the visual start of
"Happy birthday!<BOM>").

Root cause: RTL_REGEX conflated two unrelated concepts — real RTL letter
codepoints (Hebrew, Arabic, Syriac, etc.) and invisible BiDi formatting
controls
. include_rtl? matched either, so a string containing only stray
clipboard artifacts — no actual RTL letters — incorrectly took the reorder path
in PrawnArrangerPatch#finalize_line.

Fix

Narrow the detection heuristic. No text is stripped or mutated — only the
trigger for the reorder path changes.

Split the single RTL_REGEX into three constants:

  • RTL_LETTERS_REGEX — real RTL letter scripts only (֐–ࣿ,
    יִ–﷿, ﹰ–﻾). Used by include_rtl? as the sole guard
    for entering the reorder path.
  • RTL_FORMAT_CONTROLS_REGEX — the BiDi format controls (, ,
    ‪–‮, ⁦–⁩). Exposed for external callers but no longer
    used for reorder detection.
  • RTL_REGEXRegexp.union of both. Back-compat alias; no internal consumer
    references it directly.
Input Old include_rtl? New include_rtl? Result
"Happy birthday!" (bug case) true false fixed
"مرحبا" (pure Arabic) true true unchanged
Arabic + LRM + digits true true digit order preserved, LRM intact
Stray isolate alone true false no visual change

Genuine Arabic/Hebrew text with embedded LRM/RLM/isolates that disambiguate
digit ordering still matches on its letter codepoints, goes through reorder,
and has all formatting controls left intact in the string — nothing is
stripped.

Tests

12 specs in spec/prawn/rtl/connector_spec.rb:

  • include_rtl?false for stray-control-only strings: BOM, LRM, RLM,
    LRE, RLI each in isolation.
  • include_rtl?true for Hebrew, Arabic, Arabic Presentation Forms.
  • include_rtl?true for mixed Arabic + LRM + digits.
  • fix_rtl → no-op for "Happy birthday!" (the original bug case).

spec/spec_helper.rb adds require "prawn" so Prawn::Text is defined
before the gem's prepend calls run.

The pre-existing connect arabic string and reverse spec was failing since
09ad656 split glyph shaping out of fix_rtl. Fixed by updating the test
to invoke the full pipeline explicitly (connect_arabic then fix_rtl),
mirroring how Prawn invokes the gem in production. Renamed to
shapes Arabic glyphs and reorders RTL visually. All 12 specs now pass.

Monolith

Related: ROPE-3815 in tablecheck/monolith. After this merges to
line-wrap-support, the monolith's gemfiles/Gemfile.shared ref will be
updated to the merged SHA.

Comment thread lib/prawn/rtl/connector.rb Outdated
# NKo (07C0-07FF), Samaritan (0800-083F), Mandaic (0840-085F),
# other extended RTL characters, and Bidi control characters.
RTL_REGEX = /[\u0590-\u08FF\uFB1D-\uFDFF\uFE70-\uFEFF\u200F\u202A-\u202E\u2066-\u2069]/
RTL_REGEX = /[֐-ࣿיִ-﷿ﹰ-‏‪-‮⁦-⁩]/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

don't remove unicode escaped chars

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.

Just to be explicitly clear @johnnyshields, i've addressed this feedback. let me know if you have any other comment here or if it's okay to resolve the conversation and approve the PR 🙇

RTL_REGEX previously matched both RTL letter codepoints and invisible BiDi
formatting controls (BOM U+FEFF, RLM U+200F, embedding/override U+202A-U+202E,
isolates U+2066-U+2069). This caused strings containing only stray clipboard
artifacts — no actual Arabic/Hebrew — to enter the BiDi reorder path in
PrawnArrangerPatch#finalize_line, producing scrambled PDF output (e.g. the
neutral '!' glyph pulled to the visual start of "Happy birthday!<BOM>").

Split RTL_REGEX into:
- RTL_LETTERS_REGEX: real RTL letter scripts only (U+0590-08FF, FB1D-FDFF,
  FE70-FEFE). Used by include_rtl? — the sole guard for entering reorder.
- RTL_FORMAT_CONTROLS_REGEX: the BiDi format controls, exposed for external
  callers but no longer used for reorder detection.
- RTL_REGEX: Regexp.union of both (back-compat alias; no internal consumer
  references it directly).

This preserves both behaviours: stray controls in LTR text no longer trigger
reorder; genuine Arabic/Hebrew still matches on letter codepoints, and any
embedded LRM/RLM/isolates that carry semantic meaning for digit ordering are
left intact in the string — they are never stripped.
@wahidfarid
wahidfarid force-pushed the rope-3815-strip-bidi-formatting branch from e9e797a to 4ce9769 Compare April 24, 2026 07:10
@wahidfarid wahidfarid changed the title Strip BiDi formatting controls before Prawn text layout (v0.1.8) Narrow include_rtl? to RTL letter scripts only (v0.1.8) May 12, 2026
Add .github/workflows/ci.yml running rspec against Ruby 3.1, 3.2, 3.3 on
push and pull_request. Fix the connect arabic string and reverse spec,
which
has been failing since 09ad656 split glyph shaping out of fix_rtl:
update
it to call connect_arabic then fix_rtl explicitly, mirroring the
production
pipeline.
Comment thread Gemfile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants