Page MenuHomePhabricator

Support diffs copy and paste for RTL languages
Open, Needs TriagePublic2 Estimated Story PointsBUG REPORT

Description

What is the problem?

I am struggling to get some of the changes made for https://phabricator.wikimedia.org/tag/mediawiki-page-diffs/ working on an RTL wiki.

For example, trying to select text from one side of the diff actually selects it from the other (the feature introduced in T285793).

I also notice that diff-right and diff-left attributes (from T285776) are not flipped in RTL.

I have tried setting the user's interface language and $wgLanguageCode to ar.

This is on my local bare-metal machine with wikidiff2 1.12.0 installed (compiled locally).

Environment

Wiki(s): local wiki MediaWiki 1.37.0-alpha (930841b) 15:14, 1 September 2021 with wikidiff 1.12.0.

Event Timeline

Thanks @dom_walden!

For example, trying to select text from one side of the diff actually selects it from the other (the feature introduced in T285793).

This is bad, and I'm working on a fix. I think it should be possible to fix this in mw core alone, without touching wikidiff2.

I also notice that diff-right and diff-left attributes (from T285776) are not flipped in RTL.

I think this is just a naming issue -- "left" here just stands for "deleted", "right" for "added". The classes could be renamed without any issue (perhaps "diff-side-added" and "diff-side-deleted"). This requires a change in both wikidiff2 and mw core.

I have tried setting the user's interface language and $wgLanguageCode to ar.
This is on my local bare-metal machine with wikidiff2 1.12.0 installed (compiled locally).

FTR, I can reproduce the same issue with the "vanilla" diff formatter (i.e. without wikidiff2), using ?uselang=ar.

Amire80 renamed this task from Support for RTL languages to Support diffs copy and paste for RTL languages.Sep 16 2021, 1:33 PM
Daimona set the point value for this task to 2.Sep 16 2021, 1:36 PM

So... The diff code itself would work just fine on RTL wikis, since as I said above, "left" and "right" are just a (admittedly subpar) naming scheme, but don't actually expect things to be on the left/right. The real issue is that MW uses cssjanus to adapt stylesheets to RTL. The following style (which is the core of the diff-locking thing):

.diff[ data-selected-side='left' ] .diff-right,
.diff[ data-selected-side='right' ] .diff-left {
	-webkit-user-select: none;
	-moz-user-select: none;
	-ms-user-select: none;
	user-select: none;
	cursor: text;
}

is flipped by cssjanus in the following way:

/* Lock selection to a single side */
.diff[ data-selected-side='right' ] .diff-right,
.diff[ data-selected-side='left' ] .diff-left {
	-webkit-user-select: none;
	-moz-user-select: none;
	-ms-user-select: none;
	user-select: none;
	cursor: text;
}

i.e. the value of the "data-selected-side" attribute is switched, but not the diff-right/diff-left classes, hence making text impossible to select.

Renaming the classes and the attribute will fix everything.

Change 721524 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Rename diff-related classes for LTR compatibility

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

Change 721547 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/php/wikidiff2@master] Rename diff-related classes for LTR compatibility

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

Change 721531 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@REL1_37] Rename diff-related classes for LTR compatibility

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

Change 721524 merged by jenkins-bot:

[mediawiki/core@master] Rename diff-related classes for LTR compatibility

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

Change 721531 merged by jenkins-bot:

[mediawiki/core@REL1_37] Rename diff-related classes for LTR compatibility

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

Change 721547 merged by jenkins-bot:

[mediawiki/php/wikidiff2@master] Rename diff-related classes for LTR compatibility

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

Whoops, seems like I forgot to move this to QA.