Bug 220241 – Web Inspector: RTL content inside elements is reversed and unreadable

• Component: Web Inspector
• Status: RESOLVED FIXED

[Ebrahim Byagowi]

Created attachment 416940 [details]
RTL text inside the element is reversed and is unreadable

The fix in https://bugs.webkit.org/show_bug.cgi?id=207214 has caused Web Inspector to display RTL text completely reversed making RTL text content used inside elements completely unreadable. There is fortunately a solution to this that won't regress the fix there and to use "unicode-bidi: isolate" instead "unicode-bidi: isolate-override;" and I've applied the fix locally checking whether it won't regress the original issue and also fixes this disturbing issue.

Test page used in the attachment to show the issue:

  data:text/html;charset=utf8,سیب

"unicode-bidi: bidi-override" (and now the recently introduced isolate-override) is really something of past and was designed to make compatibility with text written before bidi and unicode era and is something should be and is avoided by modern web development.

[Ebrahim Byagowi]

Created attachment 416942 [details]
Patch

[Blaze Burg]

(In reply to Ebrahim Byagowi from comment #0)
> Created attachment 416940 [details]
> RTL text inside the element is reversed and is unreadable
> 
> The fix in https://bugs.webkit.org/show_bug.cgi?id=207214 has caused Web
> Inspector to display RTL text completely reversed making RTL text content
> used inside elements completely unreadable. There is fortunately a solution
> to this that won't regress the fix there and to use "unicode-bidi: isolate"
> instead "unicode-bidi: isolate-override;" and I've applied the fix locally
> checking whether it won't regress the original issue and also fixes this
> disturbing issue.
> 
> Test page used in the attachment to show the issue:
> 
>   data:text/html;charset=utf8,سیب
> 
> "unicode-bidi: bidi-override" (and now the recently introduced
> isolate-override) is really something of past and was designed to make
> compatibility with text written before bidi and unicode era and is something
> should be and is avoided by modern web development.

Thanks for investigating this. I noticed that Web Inspector is in LTR layout mode in your screenshots. That's fine, but could you please ensure that the behavior is also correct when in RTL layout mode?

[Radar WebKit Bug Importer]

<rdar://problem/72817371>

[Ebrahim Byagowi]

> Thanks for investigating this. I noticed that Web Inspector is in LTR layout mode in your screenshots. That's fine, but could you please ensure that the behavior is also correct when in RTL layout mode?

Thank you for reaching to this :) Does Web Inspector even have a RTL mode? This fix won't have anything to do with it AFAICT still how can I run Web Inspector in RTL mode? I am using the run-minibrowser for testings and changing system language didn't have any apparent effect to it.

[Myles C. Maxfield]

If you set your system language to Arabic and restart Safari, there should be a big change. See the screenshot I attached.

You're right that RTL text looks broken when the inspector is in LTR mode. We just want to make sure that this patch doesn't regress behavior when the inspector is in RTL mode.

[Myles C. Maxfield]

Created attachment 417483 [details]
Screenshot

[Sam Sneddon [:gsnedders]]

We previously fixed a similar issue in bug 170298 by adding bdo=auto to the DOM tree (which in the UA stylesheet adds unicode-bidi: isolate).

Bug 207214, which introduced this regression, was outright wrong using isolate-override (it only makes sense in combination with an explicitly set direction, which we don't have for page content). In general, using the dir attribute (and potentially the bdo/bdi elements) is recommended given it's generally more obvious about what it does.

There's probably a strong argument that if the page has <span dir=rtl>foo</span> then we want to end up with <span class="html-text-node" dir=rtl>foo</span> in the Inspector, rather than using auto there. That said, with the patch attached to this bug, we're in a better place than before bug 207214, so I'm a non-reviewer r+ on this.

[Blaze Burg]

Comment on attachment 416942 [details]
Patch

r=me, thanks for the help everyone.

[EWS]

Committed r271458: <https://trac.webkit.org/changeset/271458>

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