Narrow include_rtl? to RTL letter scripts only (v0.1.8) - #2
Open
wahidfarid wants to merge 4 commits into
Open
Conversation
| # 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 = /[-ࣿיִ-﷿ﹰ---]/ |
There was a problem hiding this comment.
don't remove unicode escaped chars
Author
There was a problem hiding this comment.
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
force-pushed
the
rope-3815-strip-bidi-formatting
branch
from
April 24, 2026 07:10
e9e797a to
4ce9769
Compare
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.
wahidfarid
commented
May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Stray invisible BiDi formatting controls — BOM (
U+FEFF), RLM (U+200F),embedding/override (
U+202A–U+202E), and isolates (U+2066–U+2069) — pastedfrom 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_REGEXconflated two unrelated concepts — real RTL lettercodepoints (Hebrew, Arabic, Syriac, etc.) and invisible BiDi formatting
controls.
include_rtl?matched either, so a string containing only strayclipboard 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_REGEXinto three constants:RTL_LETTERS_REGEX— real RTL letter scripts only (–ࣿ,יִ–﷿,ﹰ–). Used byinclude_rtl?as the sole guardfor entering the reorder path.
RTL_FORMAT_CONTROLS_REGEX— the BiDi format controls (,,–,–). Exposed for external callers but no longerused for reorder detection.
RTL_REGEX—Regexp.unionof both. Back-compat alias; no internal consumerreferences it directly.
include_rtl?include_rtl?"Happy birthday!"(bug case)truefalse"مرحبا"(pure Arabic)truetruetruetruetruefalseGenuine 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?→falsefor stray-control-only strings: BOM, LRM, RLM,LRE, RLI each in isolation.
include_rtl?→truefor Hebrew, Arabic, Arabic Presentation Forms.include_rtl?→truefor mixed Arabic + LRM + digits.fix_rtl→ no-op for"Happy birthday!"(the original bug case).spec/spec_helper.rbaddsrequire "prawn"soPrawn::Textis definedbefore the gem's
prependcalls run.The pre-existing
connect arabic string and reversespec was failing since09ad656split glyph shaping out offix_rtl. Fixed by updating the testto invoke the full pipeline explicitly (
connect_arabicthenfix_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 toline-wrap-support, the monolith'sgemfiles/Gemfile.sharedref will beupdated to the merged SHA.