Change 663067 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalWatchlist@master] Restore RTL handling for non-Vue display

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

@DannyS712, you've asked me to review the code, but I'm not a developer on that level. May I call to a real developer that understands a lot in RTL?

In T274313#6816958, @IKhitron wrote:

@DannyS712, you've asked me to review the code, but I'm not a developer on that level. May I call to a real developer that understands a lot in RTL?

Sure, but I was mostly just cc-ing you - its essentially copied from the user script version

So, you don't need me for +1? If so, there is no need to call him.

In T274313#6816981, @IKhitron wrote:

So, you don't need me for +1? If so, there is no need to call him.

No harm if you want to review yourself (or have someone else do so) but since this was essentially copied from the user script it is known to work already

I don't. I know it works, it's my code. I just was affraid you can't proceed to the deployment without enough reviews.

In T274313#6816987, @IKhitron wrote:

I don't. I know it works, it's my code. I just was affraid you can't proceed to the deployment without enough reviews.

+1 is never a bad thing, but only +2 is enough to merge, and only one +2 review is needed

I see. Well, I haven't +2, of course, so I'll just take a look to the code and wait.

Well, @DannyS712, I think the patch will fix the problem of rtl wikis on ltr language page.
But I do not believe it will fix the problem of ltr wikis on rtl language page.
Please open the watchlist with uselang=he. As you can see, all the wikis are aligned to right. It should not happen without the patch. Maybe there is a problem with css selectors on the line 62, thay must align all the text to the left without the patch.

In T274313#6817023, @IKhitron wrote:

Well, @DannyS712, I think the patch will fix the problem of rtl wikis on ltr language page.
But I do not believe it will fix the problem of ltr wikis on rtl language page.
Please open the watchlist with uselang=he. As you can see, all the wikis are aligned to right. It should not happen without the patch. Maybe there is a problem with css selectors on the line 62, thay must align all the text to the left without the patch.

Can you leave such reviews on the commit?
Also, does this affect the user script version?

I do not know how to do this, never tryed.
The script works fine for both cases, up to now.
Unfortunately I can't check the selectors using F12 tools by myself, probably until Thursday evening.

Well, @DannyS712, I still have no normal F12 tools, but try to replace the line 62 selector with

.ext-globalwatchlist-feed-site

In T274313#6817312, @IKhitron wrote:

Well, @DannyS712, I still have no normal F12 tools, but try to replace the line 62 selector with

.ext-globalwatchlist-feed-site

Done in patchset 3

In T274313#6817494, @DannyS712 wrote:

In T274313#6817312, @IKhitron wrote:

Well, @DannyS712, I still have no normal F12 tools, but try to replace the line 62 selector with

.ext-globalwatchlist-feed-site

Done in patchset 3

Great. Are you going to merge it today on testwiki, do we can check if it works?

In T274313#6818122, @IKhitron wrote:

In T274313#6817494, @DannyS712 wrote:

In T274313#6817312, @IKhitron wrote:

Well, @DannyS712, I still have no normal F12 tools, but try to replace the line 62 selector with

.ext-globalwatchlist-feed-site

Done in patchset 3

Great. Are you going to merge it today on testwiki, do we can check if it works?

I can't merge it, since its my patch, but once its merged it can be tested on the beta cluster to confirm whether it works, and if it is we backport to testwiki.

In T274313#6818502, @DannyS712 wrote:

I can't merge it, since its my patch, but once its merged it can be tested on the beta cluster to confirm whether it works, and if it is we backport to testwiki.

If it could be tested on beta cluster, I would find the problem much earlier. We need at least testwiki to get data from both ltr and rtl wikis.

In T274313#6818505, @IKhitron wrote:

In T274313#6818502, @DannyS712 wrote:

I can't merge it, since its my patch, but once its merged it can be tested on the beta cluster to confirm whether it works, and if it is we backport to testwiki.

If it could be tested on beta cluster, I would find the problem much earlier. We need at least testwiki to get data from both ltr and rtl wikis.

See https://meta.wikimedia.beta.wmflabs.org/wiki/Special:SiteMatrix - there are hebrew and arabic testwiki, so it should be possible to test there

In T274313#6818940, @DannyS712 wrote:

See https://meta.wikimedia.beta.wmflabs.org/wiki/Special:SiteMatrix - there are hebrew and arabic testwiki, so it should be possible to test there

Thanks. There is no hebrew wiki, it's just a stub, but I've managed to use the arabic one, and the english one as well. The uselang parameter also works. I've prepared everything, and I can check the result indeed after the merge on beta.

Change 663067 merged by jenkins-bot:
[mediawiki/extensions/GlobalWatchlist@master] Restore RTL handling for non-Vue display

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

In T274313#6819044, @IKhitron wrote:

In T274313#6818940, @DannyS712 wrote:

See https://meta.wikimedia.beta.wmflabs.org/wiki/Special:SiteMatrix - there are hebrew and arabic testwiki, so it should be possible to test there

Thanks. There is no hebrew wiki, it's just a stub, but I've managed to use the arabic one, and the english one as well. The uselang parameter also works. I've prepared everything, and I can check the result indeed after the merge on beta.

Its merged, should be live on the beta cluster to test (check https://meta.wikimedia.beta.wmflabs.org/wiki/Special:Version under global watchlist what the version is, it'll link to the last merged change that is deployed)

Thanks @DannyS712 and @Legoktm. It was definitely merged, and it's much better now, but not enough.
See how it looks on LTR wikis:

Ack. So I think the CSS solution just isn't going to work, instead I think we should identify whether the remote wiki is LTR or RTL, and then wrap the contents in <div dir="ltr"> or <div dir="rtl"> accordingly.

Now that I typed it, I'm not sure looking at the directionality of the wiki is right, shouldn't we look at the directionality of the user's chosen interface language on that wiki? My interface language is "English", so I would expect my watchlist to be LTR, even on RTL wikis.

In T274313#6821779, @Legoktm wrote:

Ack. So I think the CSS solution just isn't going to work, instead I think we should identify whether the remote wiki is LTR or RTL, and then wrap the contents in <div dir="ltr"> or <div dir="rtl"> accordingly.

Now that I typed it, I'm not sure looking at the directionality of the wiki is right, shouldn't we look at the directionality of the user's chosen interface language on that wiki? My interface language is "English", so I would expect my watchlist to be LTR, even on RTL wikis.

For LTR interface/uselang it should be LTR, for RTL it should be RTL.
A couple of hours from now I will play with all this, trying to find a working solution.

Found the problem, trying to solve.

Well, can you try to change the code to

.ext-globalwatchlist-feed-site {
	/* @noflip */ text-align: left;
	/* @noflip */ direction: ltr;
}
/* Handling of rtl wikis */
div[ id^='ext-globalwatchlist-feed-site-he_' ],
div[ id^='ext-globalwatchlist-feed-site-ar_' ],
div[ id^='ext-globalwatchlist-feed-site-azb_' ],
div[ id^='ext-globalwatchlist-feed-site-ckb_' ],
div[ id^='ext-globalwatchlist-feed-site-dv_' ],
div[ id^='ext-globalwatchlist-feed-site-fa_' ],
div[ id^='ext-globalwatchlist-feed-site-glk_' ],
div[ id^='ext-globalwatchlist-feed-site-ks_' ],
div[ id^='ext-globalwatchlist-feed-site-lrc_' ],
div[ id^='ext-globalwatchlist-feed-site-mzn_' ],
div[ id^='ext-globalwatchlist-feed-site-nqo_' ],
div[ id^='ext-globalwatchlist-feed-site-pnb_' ],
div[ id^='ext-globalwatchlist-feed-site-ps_' ],
div[ id^='ext-globalwatchlist-feed-site-sd_' ],
div[ id^='ext-globalwatchlist-feed-site-ug_' ],
div[ id^='ext-globalwatchlist-feed-site-ur_' ],
div[ id^='ext-globalwatchlist-feed-site-yi_' ] {
	/* @noflip */ text-align: right;
	/* @noflip */ direction: rtl;
}

?

In T274313#6822818, @IKhitron wrote:

Well, can you try to change the code to

.ext-globalwatchlist-feed-site {
	/* @noflip */ text-align: left;
	/* @noflip */ direction: ltr;
}
/* Handling of rtl wikis */
div[ id^='ext-globalwatchlist-feed-site-he_' ],
div[ id^='ext-globalwatchlist-feed-site-ar_' ],
div[ id^='ext-globalwatchlist-feed-site-azb_' ],
div[ id^='ext-globalwatchlist-feed-site-ckb_' ],
div[ id^='ext-globalwatchlist-feed-site-dv_' ],
div[ id^='ext-globalwatchlist-feed-site-fa_' ],
div[ id^='ext-globalwatchlist-feed-site-glk_' ],
div[ id^='ext-globalwatchlist-feed-site-ks_' ],
div[ id^='ext-globalwatchlist-feed-site-lrc_' ],
div[ id^='ext-globalwatchlist-feed-site-mzn_' ],
div[ id^='ext-globalwatchlist-feed-site-nqo_' ],
div[ id^='ext-globalwatchlist-feed-site-pnb_' ],
div[ id^='ext-globalwatchlist-feed-site-ps_' ],
div[ id^='ext-globalwatchlist-feed-site-sd_' ],
div[ id^='ext-globalwatchlist-feed-site-ug_' ],
div[ id^='ext-globalwatchlist-feed-site-ur_' ],
div[ id^='ext-globalwatchlist-feed-site-yi_' ] {
	/* @noflip */ text-align: right;
	/* @noflip */ direction: rtl;
}

?

I didn't think about the interaction with CSSJanus flipping stuff, since that wasn't the case for the user script.

In T274313#6823802, @DannyS712 wrote:

I didn't think about the interaction with CSSJanus flipping stuff, since that wasn't the case for the user script.

So, can you do this?

Change 663393 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalWatchlist@master] Add @noflip commands for CSS

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

Change 663393 merged by jenkins-bot:
[mediawiki/extensions/GlobalWatchlist@master] Add @noflip commands for CSS

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

Change 663397 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalWatchlist@wmf/1.36.0-wmf.30] Restore RTL handling for non-Vue display

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

Change 663399 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalWatchlist@wmf/1.36.0-wmf.30] Add @noflip commands for CSS

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

@DannyS712, I'm here, but where is the jenkins-bot?

In T274313#6824487, @IKhitron wrote:

@DannyS712, I'm here, but where is the jenkins-bot?

The beta cluster now shows the result with the @noflip comments added - does this look good to you?

Are you sure? The jenkins-bot did not leave a comment this time to say it's already there, and nothing changed visually.

In T274313#6824558, @IKhitron wrote:

Are you sure? The jenkins-bot did not leave a comment this time to say it's already there, and nothing changed visually.

Its changed visually for me - try https://meta.wikimedia.beta.wmflabs.org/wiki/Special:GlobalWatchlist?uselang=he&debug=true

Maybe I needed a deep refresh once more. It works fine now in all cases.
On LTR interface:

In that case, these changes will be backported - thanks for confirming

Change 663397 merged by jenkins-bot:
[mediawiki/extensions/GlobalWatchlist@wmf/1.36.0-wmf.30] Restore RTL handling for non-Vue display

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

Change 663399 merged by jenkins-bot:
[mediawiki/extensions/GlobalWatchlist@wmf/1.36.0-wmf.30] Add @noflip commands for CSS

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

If I understand properly, I should see the results on testwiki now?

In T274313#6824869, @IKhitron wrote:

If I understand properly, I should see the results on testwiki now?

Yes - though if your javascript is cached, adding ?debug=true to the url should fix it

Confirm that it works on testwiki.