Skip to content

Fix RTL plugin crash with empty formatted text sections#6545

Closed
Omkarthipparthi wants to merge 3 commits intomaplibre:mainfrom
Omkarthipparthi:fix-rtl-empty-formatted-sections
Closed

Fix RTL plugin crash with empty formatted text sections#6545
Omkarthipparthi wants to merge 3 commits intomaplibre:mainfrom
Omkarthipparthi:fix-rtl-empty-formatted-sections

Conversation

@Omkarthipparthi
Copy link
Copy Markdown
Contributor

@Omkarthipparthi Omkarthipparthi commented Oct 12, 2025

Description

Fixes crash when using RTL text plugin with formatted text-field expressions where all sections evaluate to empty strings.

Closes #6444

Problem

When using the RTL text plugin (@mapbox/mapbox-gl-rtl-text@0.3.0) with a formatted text-field where all sections are empty (e.g., both name and ref are null), the application crashes with:

Root Cause

The RTL plugin's processStyledBidirectionalText can return either:

  • Tuples: [text, sectionIndex] (expected format)
  • Objects: {text, sectionIndex} (some plugin versions/edge cases)

The shaping code in src/symbol/shaping.ts only handled tuples, causing taggedLine.text to be undefined when objects were returned, leading to crashes when later code accessed this.text.length.

Solution

Added defensive normalization logic (lines 331-361 in shaping.ts) to:

  • Accept both tuple [text, sectionIndex] and object {text, sectionIndex} formats
  • Provide safe fallbacks: empty string '' for text, empty array [] for sectionIndex
  • Maintain backward compatibility with all RTL plugin versions

Testing

  • All 17 integration shaping tests pass
  • All 2476 unit tests pass (including 10 shaping-specific tests)
  • Tested locally with @mapbox/mapbox-gl-rtl-text@0.3.0
  • No regressions in existing functionality

Test Cases Verified

  1. Empty formatted sections (the bug repro) - No crash
  2. Arabic text with multiple sections - Renders correctly
  3. Hebrew + English mixed RTL/LTR - Renders correctly
  4. Partial empty sections (only name or ref) - Handles gracefully

Checklist

@HarelM
Copy link
Copy Markdown
Collaborator

HarelM commented Oct 26, 2025

Have we tried opening an issue to the RTL plugin repo?
I think it makes sense that the API will return the same type of object and that we won't need to patch it here if possible.

@Omkarthipparthi
Copy link
Copy Markdown
Contributor Author

Omkarthipparthi commented Oct 27, 2025

@HarelM
Yes, it makes sense to fix it in the RTL plugin to have a consistent return value type. Thanks for the heads up.
I have opened an issue and its respective PR.
mapbox/mapbox-gl-rtl-text#35
mapbox/mapbox-gl-rtl-text#36

@HarelM
Copy link
Copy Markdown
Collaborator

HarelM commented Mar 20, 2026

While this might have taken a little bit of time a new version will be released soon which will solve this issue, so I'm closing this PR.
If there are any issues do let me know.

@HarelM HarelM closed this Mar 20, 2026
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.

Undefined text property for formatted text with multiple empty sections and right to left plugin

2 participants