Assertion when encountering U_OTHER_NEUTRAL in createBidiRunsForLine at end of run

[Doug Kelly]

When encountering U_OTHER_NEUTRAL in createBidiRunsForLine at the end of a run, the current case asserts because the text direction cannot be determined in the current switch case.

Example:

<font dir="ltr">a</font>
<marquee>b</marquee>

<rdar://57201879>

[Doug Kelly]

Created attachment 384119 [details]
Patch

[Ryosuke Niwa]

Waiting for EWS.

[Alexey Proskuryakov]

Comment on attachment 384119 [details]
Patch

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

> Source/WebCore/platform/text/BidiResolver.h:900
> +                        // ASSERT_NOT_REACHED();

We don't use commented out code. Please remove before landing.

[Ryosuke Niwa]

Comment on attachment 384119 [details]
Patch

Oh, that's a good point. cq- for that.

[Doug Kelly]

Created attachment 384249 [details]
Patch

[Sam Weinig]

Comment on attachment 384249 [details]
Patch

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

> Source/WebCore/platform/text/BidiResolver.h:899
> +                        // FIXME: handle neutrals in this case.

Thanks for the patch Doug. Ideally, this would have a link to the bugzilla bug filed to fix this issue.

[Doug Kelly]

Created attachment 384675 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 384675 [details]
Patch

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

> Source/WebCore/platform/text/BidiResolver.h:900
> +                        // FIXME: handle neutrals in this case.
> +                        // https://bugs.webkit.org/show_bug.cgi?id=204485

What Sam means is to file a new bug where this issue will be tracked.

[Doug Kelly]

Ah, apologies for the confusion.  I'm not sure if there's another bug we can track here, but I'll see what I can find.

[Doug Kelly]

This could be related to Bug 178960, since using ICU's bidi algorithm would likely remove the necessity for this code, but do we want a more specific bug tracking this issue?

[Sam Weinig]

(In reply to Doug Kelly from comment #10)
> This could be related to Bug 178960, since using ICU's bidi algorithm would
> likely remove the necessity for this code, but do we want a more specific
> bug tracking this issue?

Yes. There should be a new specific bug filed fix the issue identified.

[Doug Kelly]

Created attachment 384760 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 384760 [details]
Patch

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

> Source/WebCore/platform/text/BidiResolver.h:900
> +                        // FIXME: handle neutrals in this case.
> +                        // Missing cases tracked by https://bugs.webkit.org/show_bug.cgi?id=204817

You can do this in a single like: // FIXME: handle neutrals in this case. See https://webkit.org/b/204817.

[Doug Kelly]

Created attachment 384761 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 384761 [details]
Patch

Clearing flags on attachment: 384761

Committed r253068: <https://trac.webkit.org/changeset/253068>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.