Closed Bug 1428771 Opened 2 years ago Closed 2 years ago

UCS2_CHAR_IS_BIDI doesn't check for the second supplementary-plane RTL range

Categories

(Core :: Internationalization, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: hsivonen, Assigned: hsivonen)

References

Details

Attachments

(1 file)

We have
#define UTF32_CHAR_IS_BIDI(c)  ((IS_IN_BMP_RTL_BLOCK(c)) || \
                               (IS_RTL_PRESENTATION_FORM(c)) || \
                               (IS_IN_SMP_RTL_BLOCK(c)))
where
#define IS_IN_SMP_RTL_BLOCK(c) (((0x10800 <= (c)) && ((c) <= 0x10fff)) || \
                                ((0x1e800 <= (c)) && ((c) <= 0x1eFFF)))

We also have
#define UCS2_CHAR_IS_BIDI(c) ((IS_IN_BMP_RTL_BLOCK(c)) || \
                              (IS_RTL_PRESENTATION_FORM(c)) || \
                              (c) == 0xD802 || (c) == 0xD803)

(c) == 0xD802 || (c) == 0xD803 are the lead surrogates for ((0x10800 <= (c)) && ((c) <= 0x10fff)) but the lead surrogate check corresponding to ((0x1e800 <= (c)) && ((c) <= 0x1eFFF)) is missing.
nsTextBoxFrame seems to be the only user of this macro. Is there a good example I could modify to write a reftest for this?
Flags: needinfo?(jfkthame)
Blocks: 1428774
Umm... I'm not entirely sure. Maybe by putting supplementary-plane RTL text into a XUL label where one of the characters is supposed to be marked as an accessKey, but its position in the string will be computed incorrectly if we don't detect the need for bidi handling? So perhaps start from something like layout/reftests/xul/accesskey* (but not on macOS, where we don't underline the access keys).
Flags: needinfo?(jfkthame)
The macro should be renamed to UTF16_CHAR_IS_BIDI. The current name even implies that it does not support supplementary-plane characters.
(In reply to Jonathan Kew (:jfkthame) from comment #3)
> Umm... I'm not entirely sure. Maybe by putting supplementary-plane RTL text
> into a XUL label where one of the characters is supposed to be marked as an
> accessKey, but its position in the string will be computed incorrectly if we
> don't detect the need for bidi handling? So perhaps start from something
> like layout/reftests/xul/accesskey* (but not on macOS, where we don't
> underline the access keys).

Thanks. It's probably simpler not to land tests here and instead have this tested by fixing bug 1428774 and testing HTML cases after that.

(In reply to Masatoshi Kimura [:emk] from comment #4)
> The macro should be renamed to UTF16_CHAR_IS_BIDI. The current name even
> implies that it does not support supplementary-plane characters.

Good point. Renamed to UTF16_CODE_UNIT_IS_BIDI.
Comment on attachment 8940720 [details]
Bug 1428771 - Make UCS2_CHAR_IS_BIDI check for lead surrogates corresponding to U+1E800...U+1EFFF and rename to UTF16_CODE_UNIT_IS_BIDI.

https://reviewboard.mozilla.org/r/210994/#review217044
Attachment #8940720 - Flags: review?(jfkthame) → review+
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/17f0b1183b35
Make UCS2_CHAR_IS_BIDI check for lead surrogates corresponding to U+1E800...U+1EFFF and rename to UTF16_CODE_UNIT_IS_BIDI. r=jfkthame
https://hg.mozilla.org/mozilla-central/rev/17f0b1183b35
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.