Bug 220885 – WebKit doesn't automatically right-align Adlam

• Component: Text
• Status: RESOLVED FIXED

[Fuqiao Xue]

Adlam text displayed in a block element with 'dir' set to 'auto' should be automatically right-aligned, unless the base direction is set to LTR.

See https://github.com/w3c/afrlreq/issues/13 for more info.

[Smoley]

Can you please attach a test html file as well? Thanks

[Radar WebKit Bug Importer]

<rdar://problem/73688496>

[Smoley]

Actually it looks like this page may work: http://adlamtesting.appspot.com

[Smoley]

On second thought that page is showing it in a <textarea> but at least it provides some test text.

[Fuqiao Xue]

Created attachment 418695 [details]
WebKit doesn't automatically right-align Adlam

Here's a test HTML file.

[Myles C. Maxfield]

The content has dir="auto"

[Myles C. Maxfield]

If the text is moved out from the intermediate <p> element, it becomes right aligned. Somehow the <p> element is causing the problem.

[Myles C. Maxfield]

(In reply to Myles C. Maxfield from comment #7)
> If the text is moved out from the intermediate <p> element, it becomes right
> aligned. Somehow the <p> element is causing the problem.

Whoops, this is only true if you insert some RTL text like Arabic at the beginning of the string.

[Myles C. Maxfield]

(In reply to Myles C. Maxfield from comment #8)
> (In reply to Myles C. Maxfield from comment #7)
> > If the text is moved out from the intermediate <p> element, it becomes right
> > aligned. Somehow the <p> element is causing the problem.
> 
> Whoops, this is only true if you insert some RTL text like Arabic at the
> beginning of the string.

Yeah, ignore what I said about the <p> element. The <p> element has nothing to do with this.

[Myles C. Maxfield]

The first place to check would be StringImpl::defaultWritingDirection()

[Myles C. Maxfield]

oh hahahaha we're terrible

auto charDirection = u_charDirection(is8Bit() ? m_data8[i] : m_data16[i]);

super broken

[Myles C. Maxfield]

Created attachment 418778 [details]
Patch

[EWS]

Committed r272084: <https://trac.webkit.org/changeset/272084>

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

[Darin Adler]

Comment on attachment 418778 [details]
Patch

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

> Source/WTF/wtf/text/StringImpl.cpp:1672
>  UCharDirection StringImpl::defaultWritingDirection(bool* hasStrongDirectionality)

Some day to clean up: Feels to me like we should move this function to StringView or make it a non-member function that takes a StringView, in some place where we have code about writing direction and bidirectional algorithms. And give it a better interface with return values instead of an out argument. And add a fast path so we aren’t calling u_charDirection in common cases.

[Darin Adler]

Comment on attachment 418778 [details]
Patch

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

> Source/WTF/wtf/text/StringImpl.cpp:1672
>  UCharDirection StringImpl::defaultWritingDirection(bool* hasStrongDirectionality)

Some day to clean up: Feels to me like we should move this function to StringView or make it a non-member function that takes a StringView, in some place where we have code about writing direction and bidirectional algorithms. And give it a better interface with return values instead of an out argument. And add a fast path so we aren’t calling u_charDirection in common cases.

[Myles C. Maxfield]

(In reply to Darin Adler from comment #15)
> Comment on attachment 418778 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=418778&action=review
> 
> > Source/WTF/wtf/text/StringImpl.cpp:1672
> >  UCharDirection StringImpl::defaultWritingDirection(bool* hasStrongDirectionality)
> 
> Some day to clean up: Feels to me like we should move this function to
> StringView or make it a non-member function that takes a StringView, in some
> place where we have code about writing direction and bidirectional
> algorithms. And give it a better interface with return values instead of an
> out argument. And add a fast path so we aren’t calling u_charDirection in
> common cases.

Yeah, Alex and I had a conversation about the direction to move this class. Sounds like we all agree :)

[r12a]

Thanks for fixing this.