| Summary: | Check bidiLevels are valid before reordering | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brandon <brandonstewart> | ||||||
| Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, simon.fraser, webkit-bug-importer, zalan | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Other | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Brandon
2022-02-09 09:57:10 PST
Created attachment 451397 [details]
Patch
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. Created attachment 451579 [details]
Patch
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]. |