[webkit-reviews] review denied: [Bug 79749] Determine text direction for rendering a TextTrackCue : [Attachment 184299] Rebased patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Victor Carbune
<vcarbune at chromium.org>'s request for review:
Bug 79749: Determine text direction for rendering a TextTrackCue
https://bugs.webkit.org/show_bug.cgi?id=79749

Attachment 184299: Rebased patch
https://bugs.webkit.org/attachment.cgi?id=184299&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=184299&action=review


Can you help me understand what problem you're trying to solve?  This does not
seem like the right solution.

> Source/WebCore/html/track/TextTrackCue.cpp:636
> +    // ... to determine the paragraph embedding level of the first Unicode
paragraph of the cue.
> +
> +    // If the paragraph embedding level determined in the previous step is
even
> +    // (the paragraph direction is left-to-right), let direction be 'ltr',
otherwise, let it be 'rtl'.
> +    m_displayDirection = bidiRuns.firstRun()->level() % 2 ? CSSValueRtl :
CSSValueLtr;

This is a huge amount of code to answer this very simple question.  You want
basically text-direction: auto behavior it seems.

If this is the right way to do this (and I don't believe that), then this is
the wrong abstraction to use.  You shouldn't have to do a full bidi line layout
to ask what the direction of the first character is. :)

Also, Bi-di text is just that "bi directional", so it commonly has mixed LTR
and RTL.  That's why we have lots of bidi runs.  They're not going to all be
the same direction. :)