I'll happily mentor this for Google-Code-In-2016 - should be nice and easy :D

@Yurik is there any additional information which can be added to this task?

Nope, it is an easy one))

Task in GCI: https://codein.withgoogle.com/dashboard/tasks/5185098156605440/

However, the description is a bit short and requires some context (the extension, ResourceLoader, ...), which isn't a big problem, but also not very great :) My feeling is, that a 15 year old student, who contributes the first time, may be a bit overwhelmed with this task as it is :) Maybe you can add some resource where the student can find more information about the involved things in this task? Or, @Samtar, will you be very available to give a starting point for a possible student? Not a guide, but something like a bit context and a "look here, this looks like your starting point"? :D Maybe write it into the task, that the student should feel free to contact you (and probably yurik, too? :D)

@Samtar: ping - can you please reply to the last comment? This task will become available in GCI tomorrow...

Change 329605 had a related patch set uploaded (by Divadsn):
Remove /* @noflip */ Leaflet's CSS fixes

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

Change 329605 merged by jenkins-bot:
Remove /* @noflip */ Leaflet's CSS fixes

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

Approved.

As far as I can tell, the map controls now display LTR in a RTL language, which is a regression.
Try: https://en.wikipedia.beta.wmflabs.org/wiki/Map?uselang=he#/map/0

I think we should revert this patch.

Change 330220 had a related patch set uploaded (by JGirault):
Revert "Remove /* @noflip */ Leaflet's CSS fixes"

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

I cannot reproduce your issue, for me it is displaying right, so this has nothing to do with my patch.

Your example:

My example using a LTR language (Polish):

So I don't see a reason why we should revert my patch, maybe it is something with your browser wrong or with the language itself.

We need someone else to confirm, because that is what I see :

@JGirault, but what is exactly wrong? Because I see no difference, RTL is displaying correctly, so I don't understand your issue.

@Girault, yes, this also no longer flips the native 'leaflet' controls inside .leaflet-control-container anymore of course.

You could manually flip those using something like:

/* @noflip */
body.rtl .leaflet-left {
    left: initial;
    right: 0;
    .leaflet-control {
        float: right;
        margin-left: initial;
        margin-right: 10px;
    }
}
/* @noflip*/
body.rtl .leaflet-right {
    right: initial;
    left: 0;
    .leaflet-control {
        margin-right: initial;
        margin-left: 10px;
        float: left;
    }
}

Interestingly enough however, this seems to miss certain things like the the scale and attribution elements, which are actually defined in several kartographer specific stylesheets, but don't appear to get flipped either... not sure what that is about... Possibly the noflip of the one RL module get's propagated to all parent's that include it ????? That would be an oversight in T148572.

Long story short, leaflet/mapbox supports being embedded inside an rtl surface, but nothing about leaflet itself will ever do RTL by default. It's support of rtl is therefore 'mixed', and thus it requires us to always apply some adaptations, no matter if we start with a fully flipped surface (now we need to correct for the partial rtl support) or an unflipped surface (now we need to flip all the leaflet UI ourselves).

@divadsn Only the controls 'inside' the surface are wrong. The zoom controls, close button and the attribution.

Compare:

• https://en.wikipedia.beta.wmflabs.org/wiki/Map?uselang=he#/map/0
• https://en.wikipedia.org/wiki/User:Yurik/maplink?uselang=he#/maplink/0

It's not your fault, it's an oversight in the initial analysis of this problem I think.

@divadsn Yes, don't take me wrong, the patch itself is good, but it introduces a regression for our RTL users. Like @TheDJ said, I think we did not anticipate the effects correctly.

In T148695#2912881, @TheDJ wrote:

You could manually flip those using something like:

/* @noflip */
body.rtl .leaflet-left {
    left: initial;
    right: 0;
    .leaflet-control {
        float: right;
        margin-left: initial;
        margin-right: 10px;
    }
}
/* @noflip*/
body.rtl .leaflet-right {
    right: initial;
    left: 0;
    .leaflet-control {
        margin-right: initial;
        margin-left: 10px;
        float: left;
    }
}

Do you think this is better than the initial mapbox-style-fixes.css ?

In T148695#2912889, @TheDJ wrote:

@divadsn Only the controls 'inside' the surface are wrong. The zoom controls, close button and the attribution.

Compare:

• https://en.wikipedia.beta.wmflabs.org/wiki/Map?uselang=he#/map/0
• https://en.wikipedia.org/wiki/User:Yurik/maplink?uselang=he#/maplink/0

It's not your fault, it's an oversight in the initial analysis of this problem I think.

Yes, I only noticed that small difference, but I am asking myself if it does matter where these controls are. But I am not a RTL user, so I cannot say anything about that :)

In T148695#2912938, @divadsn wrote:

In T148695#2912889, @TheDJ wrote:

@divadsn Only the controls 'inside' the surface are wrong. The zoom controls, close button and the attribution.

Compare:

• https://en.wikipedia.beta.wmflabs.org/wiki/Map?uselang=he#/map/0
• https://en.wikipedia.org/wiki/User:Yurik/maplink?uselang=he#/maplink/0

It's not your fault, it's an oversight in the initial analysis of this problem I think.

Yes, I only noticed that small difference, but I am asking myself if it does matter where these controls are. But I am not a RTL user, so I cannot say anything about that :)

Some time ago we received a request T135935 from a Hebrew user (@Amire80) to flip the controls for consistency.

Maybe we should also apply noflip to RL for kartographer.less, because these controls should be flipped as I see it from this patche here: https://gerrit.wikimedia.org/r/#/c/291100/2/styles/kartographer.less

I see that someone did revert the change I mentioned before, so we should revert that one or reapply the mentioned above fixes to kartographer.less and it should be ok :)

Change 330220 merged by jenkins-bot:
Revert "Remove /* @noflip */ Leaflet's CSS fixes"

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