[webkit-reviews] review denied: [Bug 135823] Adjust max-width of cues based on text alignment when cue size is expanded : [Attachment 236418] patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has denied Roger Fong
<roger_fong at apple.com>'s request for review:
Bug 135823: Adjust max-width of cues based on text alignment when cue size is
expanded
https://bugs.webkit.org/show_bug.cgi?id=135823

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

------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=236418&action=review


This is really close, but we have to take RTL languages into account.  Take a
look at "textAlignResolvingStartAndEnd" in WebCore/editing/EditingStyle.cpp for
a snippet that shows what we  need here.

r- to fix the RTL handling, but otherwise it looks great!

> Source/WebCore/html/track/TextTrackCueGeneric.cpp:107
> +    if (alignment == CSSValueEnd || alignment == CSSValueRight)

CSSValueEnd is only the same as CSSValueRight in Left-to-Right contexts I think
we need to do something where we check for RTL, and set alignment to left/right
as needed.

> Source/WebCore/html/track/VTTCue.cpp:184
> +    if (alignment == CSSValueEnd || alignment == CSSValueRight)

Ditto my earlier comment about Start/End.