Page MenuHomePhabricator

reexamine the use of <bdi> in RevisionSlider
Closed, ResolvedPublic2 Estimated Story Points

Description

In the RevisionSlider popups I see the <bdi> tag used a lot.

It is useful for resolving certain RTL issues, but I'm not sure that it's needed in this code. For example, I see that in some instances, there is a <p> tag inside a <bdi> tag, which doesn't make sense, because <bdi> provides bidi isolation, but <p> is already isolated. Either I misunderstand the intention behind using this tag, or it helps fix some browser bugs with which I am not familiar, or it is just unnecessary :)

I'll be happy to help clean its usage up.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 301077 had a related patch set uploaded (by WMDE-leszek):
Remove not needed <bdi> tags

https://gerrit.wikimedia.org/r/301077

Thank you @Amire80 for pointing this out.
I believe this the last of reasons you listed: those tags are remainders of some previous version of the code.

I personally don't feel really confident with RTL issues, so your help with cleaning those tags up would be very appreciated.
As all usages of <bdi> tag seem to only wrap a <p> tag, I submitted https://gerrit.wikimedia.org/r/#/c/301077/ which simply removes them.

Combinations of RTL-label (e.g. username, comment) and LTR-value look OK for me if I switch me local wiki to Hebrew, as far as I can tell not being able to read Hebrew. I am not sure if we still should have added those <bidi> tags somewhere, though. For instance, does it makes sense to add <bidi> tag around the value as it might need be displayed using different direction than label.

What I mean is this the way it should be?

<p><strong>Username:</strong> <bidi><a href="link to user page">HEBREW USERNAME</a></bidi></p>

Currently there is no <bidi> tags around username/summary, although as said my Firefox is either smart enough to display those combinations properly, or it is just not needed to help browser with <bidi> tags in such cases.

To make it short: It would be lovely if you could have a look at this patch on gerrit. Please comment there if you think simple removal is not enough/not right. Hopefully we will get this cleaned up soon!

WMDE-leszek moved this task from Proposed to Review on the TCB-Team-Sprint-2016-07-14 board.

First: it's <bdi>, not <bidi>. It mean "bi-directional isolation".

Other than that, yes: putting the <bdi> tag around usernames, titles and edit summaries indeed makes a lot of sense. Something like:

<p><strong>Username:</strong> <bdi><a href="link to user page">HEBREW USERNAME</a></bdi></p>

(Same thing as you wrote, but <bdi> instead of <bidi>.)

For general information, <bdi> is almost the same thing as <span>—just a generic inline element (not a block element). The difference is that it isolates the text inside it from adjacent elements, so that elements would not "climb" onto each other, as it sometimes happens in mixed LTR-RTL texts. <bdi> should be used instead of <span> when you expect the possibility of appearance of text in a different language, and with titles, usernames and edit summaries this definitely can happen.

First: it's <bdi>, not <bidi>. It mean "bi-directional isolation".

Right, I haven't woken up completely yet, apparently! I meant <bdi> every time I wrote <bidi>.

For general information, <bdi> is almost the same thing as <span>—just a generic inline element (not a block element). The difference is that it isolates the text inside it from adjacent elements, so that elements would not "climb" onto each other, as it sometimes happens in mixed LTR-RTL texts. <bdi> should be used instead of <span> when you expect the possibility of appearance of text in a different language, and with titles, usernames and edit summaries this definitely can happen.

That was the way I was understanding it. So we have been doing it partly wrong so far.

Other than that, yes: putting the <bdi> tag around usernames, titles and edit summaries indeed makes a lot of sense. Something like:
<p><strong>Username:</strong> <bdi><a href="link to user page">HEBREW USERNAME</a></bdi></p>

I am going to make in the patch linked above. Thanks!

Change 301077 merged by jenkins-bot:
Clean up the usage of <bdi> tags

https://gerrit.wikimedia.org/r/301077

Addshore subscribed.

Will be deployed this week.