tor circuit display has text positioning/alignment problems for RTL languages

Reported by Sherief: 

added GeorgKoppen201503R TorBrowserTeam201503R component::applications/tor browser owner::tbb-team priority::medium resolution::fixed status::closed tbb-circuit-display tbb-usability type::defect labels

Trac:

My patch for this ticket is here: https://github.com/arthuredelstein/torbutton/commit/c1a3acb3716b7c5a678bb81c8576ab37302fab70

Here's the circuit display from the Farsi Tor Browser running the patch:

Trac:
Status: new to needs_review
Keywords: N/A deleted, TorBrowserTeam201503R added

Trac:

Replying to arthuredelstein:

My patch for this ticket is here: https://github.com/arthuredelstein/torbutton/commit/c1a3acb3716b7c5a678bb81c8576ab37302fab70

This patch has the added benefit that it is able to accommodate circuits with more relays. For example, here's a circuit display with Tor Browser connected to a tor instance patched to use 4 relays:

Trac:
Keywords: N/A deleted, GeorgKoppen201503R added

Looks good. I'd like to get two nits fixed:

// Updates the Tor circuit display SVG, showing the current domain

It is no SVG anymore as far as I can see.

2. I agree that this bug fixes #13074 (moved) as well. If you have that fix in the same commit (which is fine for me) please give a hint in the commit message as well. This makes it a lot easier for people doing the release to get the changelog right (they can then just look for bug numbers with |git log|)

I tested a while the ar version and it behaves weird. (See attachment) Not sure if that is something we can/should fix, though. I guess there is an RTL/LTR mixup causing this due to yet untranslated strings?

Trac:
Status: needs_review to needs_revision

Trac:

Trac:

Trac:

Trac:

Replying to gk:

Looks good. I'd like to get two nits fixed:

{{{ // Updates the Tor circuit display SVG, showing the current domain }}} It is no SVG anymore as far as I can see.

Fixed.

2. I agree that this bug fixes #13074 (moved) as well. If you have that fix in the same commit (which is fine for me) please give a hint in the commit message as well. This makes it a lot easier for people doing the release to get the changelog right (they can then just look for bug numbers with |git log|)

Good idea. I've added a mention of #13704 (moved).

I tested a while the ar version and it behaves weird. (See attachment) Not sure if that is something we can/should fix, though. I guess there is an RTL/LTR mixup causing this due to yet untranslated strings?

Yes, it's partly due to untranslated strings. The circuit diagram should turn around if everything is translated. You're right that the parentheses are misbehaving, though, so I've added a fix for those.

Here is the new patch: https://github.com/arthuredelstein/torbutton/commit/ac63db8d95fce5f743c04a8c4f5cca7b852504e8

Here is the corrected circuit display in English, Arabic, and Farsi:

Trac:
Status: needs_revision to needs_review

Replying to arthuredelstein:

Replying to gk:

I tested a while the ar version and it behaves weird. (See attachment) Not sure if that is something we can/should fix, though. I guess there is an RTL/LTR mixup causing this due to yet untranslated strings?

Yes, it's partly due to untranslated strings. The circuit diagram should turn around if everything is translated. You're right that the parentheses are misbehaving, though, so I've added a fix for those.

Here is the new patch: https://github.com/arthuredelstein/torbutton/commit/ac63db8d95fce5f743c04a8c4f5cca7b852504e8

Neat. Could you add a comment explaining what is going on and what we are doing about it?

Replying to gk:

Neat. Could you add a comment explaining what is going on and what we are doing about it? Good idea. I added comments to the lines in question, and rebased on top of master: https://github.com/arthuredelstein/torbutton/commit/e8050135513b16792f97de4c127fe1f412f4bdff

Thanks. Merged as commit 6d0c5e64ce8e104e3f9bfe648356a4f40fdde404. I slightly updated your commit message to make it better readable on some terminals (see: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html for my reasoning).

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

mentioned in issue #15169 (moved)

moved to tpo/applications/tor-browser#15086 (closed)