Page MenuHomePhabricator

Edit box directionality cannot be changed on RTL projects
Closed, ResolvedPublic

Description

If you have configured Windows to enable both a RTL and a LTR keyboard layout (e.g. English and Persian), then you should be able to press CTRL+SHIFT to change the directionality of a textarea or textbox from RTL to LTR and back. This is a very handy feature we use on fawiki, because our wikitext may contain a mixture of text (which is RTL) and code (which would be LTR). Recently, this feature stopped working.

Originally, we suspected this may have to do with Desktop Improvements because it used to work until Desktop Improvements were turned on for fawiki. But it also has stopped working on fa.wikisource.org which still uses legacy Vector. Interestingly, it also doesn't work with Monobook anymore. Amazingly, it does work on Meta or en.wikipedia or en.wikisource

The fact that it doesn't work in both fa.wikipedia and fa.wikisource precludes it being caused by a Common.css rule (let alone the fact that I reviewed our Common.css changes and couldn't find a related change). Other RTL projects seem to also be affected (I tried he.wikipedia both logged in and logged out, and the keyboard shortcut did not work).

Also of note: the issue exists with Chrome and Firefox (have not tested other browsers), and both when logged in and logged out/incognito. This also precludes the possibility of this being caused by a browser upgrade (though I am going to install an older version of Firefox/Chrome and test with it too).

Event Timeline

Huji created this task.Aug 21 2020, 2:02 PM
Restricted Application added a project: I18n. · View Herald TranscriptAug 21 2020, 2:02 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Huji updated the task description. (Show Details)Aug 21 2020, 2:17 PM

Also interesting that adding

textarea[dir='rtl'], input[dir='rtl']
{
  direction:rtl !important;
}
textarea[dir='ltr'], input[dir='ltr']
{
  direction:ltr !important;
}

to Special:MyPage/common.css (as suggested by @Nightdevil on fawiki) does fix the issue for me.

Huji updated the task description. (Show Details)Aug 21 2020, 2:17 PM
Huji added a comment.Aug 21 2020, 2:21 PM

@Dalba it seems like the only piece of that code which you need is the following:

textarea[dir='ltr']
{
  direction:ltr !important;
}
Huji added a comment.EditedAug 21 2020, 2:24 PM

The first possible culprit I can identify by walking through browser console and mapping it to the MW code is this: 773a497a3381bc8638d002d8051db55bd86c04e7 but I don't think that is the cause, because reversing it (to the best of my ability) through browser console doesn't fix the issue.

Huji added a subscriber: Volker_E.Aug 21 2020, 2:53 PM

After realizing a mistake in how I was using git reset (i forgot to use --hard), I can now confirm that 773a497a3381bc8638d002d8051db55bd86c04e7 is the cause. Resetting to its parent d140141c07c41b226fa899e5f192ccce26db3b1b will make the issue go away.

Adding @Volker_E who is the author of 773a497 so he can opine on this.

Dalba added a comment.Aug 21 2020, 2:57 PM

@Dalba it seems like the only piece of that code which you need is the following:

textarea[dir='ltr']
{
  direction:ltr !important;
}

Yes, and It does not even need to be !important:

textarea[dir='ltr']
{
  direction:ltr;
}

Apparently textarea[dir="ltr"] selector is being overridden by .sitedir-rtl textarea. Adding the above css will help textarea[dir='ltr'] to gain back precedence.

Huji added a comment.EditedAug 21 2020, 3:13 PM

This bug only happens when those two rules are combined (as Volker_E did in that patch). When they are separate, it does not happen.

I also thought this could depend on the order of the selectors, that is, changing the top code below to the bottom code below. However, that also doesn't make the issue go away, because the browser will evaluate the entire rule together, and uses the most qualified selector all the time (in this case, the sitedir-XXX textarea supercedes the textarea[dir='XXX'] if they are both part of the same rule, no matter the order).

textarea[ dir='ltr' ],
input[ dir='ltr' ],
.sitedir-ltr textarea,
.sitedir-ltr input {
	/* @noflip */
	direction: ltr;
}

textarea[ dir='rtl' ],
input[ dir='rtl' ],
.sitedir-rtl textarea,
.sitedir-rtl input {
	/* @noflip */
	direction: rtl;
}

to

.sitedir-ltr textarea,
.sitedir-ltr input,
textarea[ dir='ltr' ],
input[ dir='ltr' ] {
	/* @noflip */
	direction: ltr;
}

.sitedir-rtl textarea,
.sitedir-rtl input,
textarea[ dir='rtl' ],
input[ dir='rtl' ] {
	/* @noflip */
	direction: rtl;
}

The only solution is to revert that change. These need to be two separate CSS rules. One of them (the one with sitedir-XXX is there to set up the defaults based on the content language. The other one (the textarea[dir='XXX'] is there to support textboxes and inputs that may or may not be in the same directionality as the content language. These two should not have been combined in the first place.

Change 621730 had a related patch set uploaded (by Huji; owner: Huji):
[mediawiki/core@master] Revert "Combine input type directionality selectors"

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

Huji claimed this task.Aug 21 2020, 3:17 PM
Huji triaged this task as Medium priority.
Huji updated the task description. (Show Details)Aug 21 2020, 3:42 PM
Demian added a subscriber: Demian.Aug 21 2020, 4:09 PM

I also thought this could depend on the order of the selectors, that is, changing the top code below to the bottom code below. However, that also doesn't make the issue go away, because the browser will evaluate the entire rule together, and uses the most qualified selector all the time (in this case, the sitedir-XXX textarea supercedes the textarea[dir='XXX'] if they are both part of the same rule, no matter the order).

input[ dir='ltr' ] and .sitedir-rtl input has the same specificity: a class and an attribute selector both add the same specificity value, therefore the order of these rules matter. Formerly the .sitedir-rtl input rule (effective on fawiki) was before the input[ dir='ltr' ], enabling the latter to override it. This was broken with the order change.

Huji added a comment.Aug 21 2020, 6:20 PM

@Demian so you think swapping to the two blocks of code like below should get it done?

.sitedir-rtl textarea,
.sitedir-rtl input,
textarea[ dir='rtl' ],
input[ dir='rtl' ] {
	/* @noflip */
	direction: rtl;
}

.sitedir-ltr textarea,
.sitedir-ltr input,
textarea[ dir='ltr' ],
input[ dir='ltr' ] {
	/* @noflip */
	direction: ltr;
}

My fear is this will fix the issue on RTL wikis, but causes the opposite issue on LTR wikis. Think of a person who wants to put a Persian poem on an enwiki page. It would be nice if they could flip the directionality of the textbox while editing the Persian poem; our current code supports that, but a combined code would not, IMHO.

Anyway, I am going to test it (and also test another suggestion @Dalba made on Gerrit) to find the middle ground, if one exists.

@Demian so you think swapping to the two blocks of code like below should get it done?

In that case RTL input on LTR site would break, as you say (inverse issue) :-)

Either the original order needs to be restored with 4 separate rules (revert, though the rules could be moved together) or the input[ dir='xxx' ] rules need to be made more specific. That can be only done by doubling the attribute selector in this case, like so:

/* Most input fields should be in site direction */
.sitedir-ltr textarea,
.sitedir-ltr input,
textarea[ dir='ltr' ][ dir='ltr' ],
input[ dir='ltr' ][ dir='ltr' ] {
	/* @noflip */
	direction: ltr;
}

.sitedir-rtl textarea,
.sitedir-rtl input,
textarea[ dir='rtl' ][ dir='rtl' ],
input[ dir='rtl' ][ dir='rtl' ] {
	/* @noflip */
	direction: rtl;
}

In this case the second [ dir='rtl' ] is redundant, as the order itself would ensure the override, but this way it is consistent (least surprise) with the [ dir='ltr' ] rules, which do need the increased specificity.

To use the Less precompiler this would be written:

textarea,
input {
	/* Most input fields should be in site direction */
	.sitedir-ltr &,
	&[ dir='ltr' ][ dir='ltr' ] {
		/* @noflip */
		direction: ltr;
	}

	.sitedir-rtl &,
	&[ dir='rtl' ][ dir='rtl' ] {
		/* @noflip */
		direction: rtl;
	}
}
Huji added a comment.Aug 21 2020, 7:32 PM

@Demian the doubling of attribute selector was a genius idea! I just tested it and it works as desired. I updated the patch.

TheDJ added a subscriber: TheDJ.EditedAug 23 2020, 3:39 PM

Yes this should be reverted (and documented) or changed to !important.

The point of this setup was, that by using a dir attribute in content areas (which does NOT necessarily have the same direction as the site), you can explicitly override both the contentdir default AND the sitedir default (normally CSS overrides attributes). This worked because the dir attribute version had an EFFECTIVE higher specificity, by being later. In the new version both stylerules always have the same specificity (0+0+1+1) and therefor you cannot use it to override any longer.

The dir attribute is what gets toggled by the ctrl+shift. This or the right click menu versions of this, work in Chrome, Safari and MS products, but not in FF (It still toggles directionality, just doesn't change the attribute value). Information btw, which the dear browser vendors might consider to document... (I just added it to MDN).

Huji added a comment.Aug 23 2020, 6:25 PM

@TheDJ you said "this should be reverted (and documented) or changed to !important". Does that mean you oppose the newer solution, in which duplicating [dir=xxx] selector results in the desired behavior?

TheDJ added a comment.Aug 23 2020, 7:19 PM

Does that mean you oppose the newer solution, in which duplicating [dir=xxx] selector results in the desired behavior?

nope.

@Huji please give proper attribution in the commit comment: T260993#6403122. Thank you.

Huji added a comment.Aug 24 2020, 12:10 AM

@Huji please give proper attribution in the commit comment: T260993#6403122. Thank you.

Done. Thanks again for the genius idea.

Done. Thanks again for the genius idea.

Thank you, @Huji.

Change 621730 merged by jenkins-bot:
[mediawiki/core@master] Properly combine input type directionality selectors

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

Huji closed this task as Resolved.Aug 26 2020, 8:24 PM