Bug 236378 - Check bidiLevels are valid before reordering
Summary: Check bidiLevels are valid before reordering
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-09 09:57 PST by Brandon
Modified: 2022-02-10 17:48 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.94 KB, patch)
2022-02-09 09:59 PST, Brandon
no flags Details | Formatted Diff | Diff
Patch (2.00 KB, patch)
2022-02-10 11:16 PST, Brandon
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brandon 2022-02-09 09:57:10 PST
<rdar://88324745>
Comment 1 Brandon 2022-02-09 09:59:43 PST
Created attachment 451397 [details]
Patch
Comment 2 zalan 2022-02-09 20:55:08 PST
Comment on attachment 451397 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=451397&action=review

> Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp:350
> +
> +            // bidiLevels are required to be less than the MAX + 1, otherwise
> +            // ubidi_reorderVisual will silently fail.
> +            if (lineRuns[i].bidiLevel() > UBIDI_MAX_EXPLICIT_LEVEL + 1)
> +                continue;
> +

Great patch! What happens here is that 
1. an empty DOM text node (length = 0) generates an InlineTextItem with zero length
2. this InlineTextItem gets ignored at InlineItemsBuilder::breakAndComputeBidiLevels() -this is ok, it's hard to assign a bidi level for a 0 length content.
3. this zero length InlineTextItem enters the bidi reordering with the default UBIDI_DEFAULT_LTR value (254, see InlineItem's c'tor) which is greater than UBIDI_MAX_EXPLICIT_LEVEL (125)
I'd slightly change this patch by adding the following asserts 
ASSERT(lineRuns[I].bidiLevel() == UBIDI_DEFAULT_LTR);
ASSERT(!downcast<InlineTextItem>(lineRuns[I]).length());
as we do not expect any other type of content with such bidi level.
Comment 3 Brandon 2022-02-10 11:16:45 PST
Created attachment 451579 [details]
Patch
Comment 4 EWS 2022-02-10 17:48:54 PST
Committed r289597 (247110@main): <https://commits.webkit.org/247110@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451579 [details].