[go: up one dir, main page]

Page MenuHomePhabricator

Remove 'setlang' query parameter feature from ULS
Closed, ResolvedPublic

Description

At the moment it is possible to change the user interface language with the URL parameter setlang in a GET request. GET should be a safe method which should not change the state of the server.[1]

Bug 49299 gets WONTFIXed by comment 1: "Given that the user will get a popup to switch back easily, and that changing the language (afaict) is just an annoyance for the user, I'm not convinced."[2]
A possibility to switch back does not make this behavior to a safe method.

I suggest to drop the URL parameter setlang completely and suggest two alternatives:

  1. Change the user interface language via API and open the page without parameter in the desired language. Described in bug 44649 and implemented in https://gerrit.wikimedia.org/r/110360 for mw.uls.changeLanguage().

The language can still changed with a single click and the new page has no troublesome URL parameter.

  1. On situations where no API call is possible use uselang instead of setlang and implement a popup to easily keep this language instead of the popup to switch back easily. The user has explicitly to confirm the change of the user setting by a single click. This is important because the link can come from a foreign page.

For a transition time the parameter setlang can have the same behavior as the parameter uselang.

[1] https://en.wikipedia.org/wiki/Hypertext_Transfer_Protocol#Safe_methods
[2] https://bugzilla.wikimedia.org/show_bug.cgi?id=49299#c1

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 2:54 AM
bzimport set Reference to bz61115.
bzimport added a subscriber: Unknown Object (MLST).

(In reply to comment #0)

  1. On situations where no API call is possible use uselang instead of setlang

and implement a popup to easily keep this language

I might misunderstand some technical details as I'm not part of language engineering, but uselang and setlang feel like two different usecases to me.
I appreciate uselang to be able to quickly and "one-time" test a problem, using values "en" or "qqx", but I'd never want to set qqx as prefered "language". ;)

(In reply to comment #1)

I appreciate uselang to be able to quickly and "one-time" test a problem,
using
values "en" or "qqx", but I'd never want to set qqx as prefered "language".

Languages that are now excluded for setlang like qqx should not present a popup to easily keep this language as default language when given as uselang. So it is not possible to set qqx as preferred language.

You can still use uselang for a quickly and "one-time" test and you can keep this language as default language with a single click on the popup.

I find the setlang parameter useful and I don't see any convincing reason to remove it. I don't consider its availability a problem. It is useful to force a language using a parameter.

Replacing its use in some places, such as mw.uls.changeLanguage() can be considered, but it should not be removed entirely.

(In reply to Amir E. Aharoni from comment #3)

I find the setlang parameter useful and I don't see any convincing reason to
remove it. I don't consider its availability a problem. It is useful to
force a language using a parameter.

setlang is a security risk. Here is a demonstrator for this risk:
https://bugzilla.wikimedia.org/attachment.cgi?id=14788

uselang allows to force a language using a parameter without a risk.

(In reply to Amir E. Aharoni from comment #3)

Replacing its use in some places, such as mw.uls.changeLanguage() can be
considered, but it should not be removed entirely.

In https://gerrit.wikimedia.org/r/110360 is a implementation for mw.uls.changeLanguage() with the same functionality without setlang. Other uses of setlang can also changed on this way.

(In reply to Fomafix from comment #4)

setlang is a security risk. Here is a demonstrator for this risk:
https://bugzilla.wikimedia.org/attachment.cgi?id=14788

Your attachment only proves that the security risk you claim is actually standard behaviour: you have two inclusions there, one of a setlang call and one of userlogout; if userlogout is acceptable, setlang is too.

if userlogout is acceptable, setlang is too.

userlogout does not change user settings but setlang changes user settings.

Legoktm subscribed.

Changing server state like this (user options) should not be possible solely using GET parameters.

Might I suggest that setlang should be changed so it doesn't actually change the language instantly but rather open a dialogue where the user can accept or deny that change? (Similar to purging a page.)

Krinkle renamed this task from Drop URL parameter setlang to Remove 'setlang' query parameter feature from ULS.Oct 11 2019, 12:43 AM
Krinkle added a project: Language-Team.

@Krinkle This is already tracked on ULS board. We tend not to use language-team tag from things that are already tracked in our product boards. If you are looking for some response with the tagging, please elaborate.

@Nikerabbit Okay, no worries. I didn't know the per-product workboards were in use as well, thought maybe the task was forgotten.

This feature is one of the very few remaining code paths that performs database changes on GET requests. In addition, due to not being behind any kind of cross-origin protection, it is also an easy abuse target through link sharing (e.g. "Read about Brexit: https://w.wiki/9rQ"), or by embedding it simply as a hidden 1px image on a random website - which would cause many to be greeted by an unintelligible interface on their next Wikipedia visit.

It would be great if we could pencil this in for Q2 (Nov) or Q3 (Feb-Mar).

Thanks for the additional info. I'll bring this up in sprint planning.

Removing setlang altogether is pretty simple. But we likely want some migration period. Some ideas to consider:

  • Add instrumentation to see how much setlang is used so we can confidently pull the plug on it.
  • Add User-notice so that the deprecation plan is announced to Wikimedia users.
  • Change the behavior and/or alert the users
  • a) If setlang is used, display popup that it will be remove in the future, if JS enabled page view.
  • b) If setlang is used, redirect to a special page that requires to submit a POST form to confirm, also message that it will be removed in the future.
  • c) Make setlang non-sticky (proposed by Krinkle). Can be combined with a popup message.

Option b) could also be a permanent solution to allow external links to suggest using a particular language. Internal code should just use the API to change the user preference.

Some cleanup is needed: https://codesearch.wmflabs.org/search/?q=setlang&i=nope&files=&repos= (LanguageSelector extension can be ignored).

Option b) could also be a permanent solution to allow external links to suggest using a particular language. Internal code should just use the API to change the user preference.

Some cleanup is needed: https://codesearch.wmflabs.org/search/?q=setlang&i=nope&files=&repos= (LanguageSelector extension can be ignored).

https://codesearch.wmflabs.org/search/?q=setlang&i=nope&files=%5C.(js%7Cphp%7Chtml)%24&repos=

Only three results then:

  1. extensions / Translate: Replaces mw.uls.changeLanguage with a function that creates a link to ?setlang= instead of using the API.
  2. extensions / UniversalLanguageSelector: Handles requests with setlang parameter.
  3. operations / mediawiki-config: A random url for fawikisource that looks like it may've confused setlang for uselang.

So the main thing is the JS method mw.uls.changeLanguage(). The default definition of that in the ULS extension uses the API already. Why is this overridden by Translate? Is this used in production?

So the main thing is the JS method mw.uls.changeLanguage(). The default definition of that in the ULS extension uses the API already. Why is this overridden by Translate? Is this used in production?

It needs to add Special:MyLanguage to the URL. I think it was overridden before ULS was changed to use the API instead. It's guarded by a configuration option that I think isn't enabled in Wikimedia production.

Did you ignore json files on purpose? I think that core message with setlang may need updating also.

abi_ triaged this task as Medium priority.

https://codesearch.wmflabs.org/search/?q=setlang&i=nope&files=%5C.(js%7Cphp%7Chtml)%24&repos=

Only three results then:

  1. extensions / Translate: Replaces mw.uls.changeLanguage with a function that creates a link to ?setlang= instead of using the API.
  2. extensions / UniversalLanguageSelector: Handles requests with setlang parameter.
  3. operations / mediawiki-config: A random url for fawikisource that looks like it may've confused setlang for uselang.

So the main thing is the JS method mw.uls.changeLanguage(). The default definition of that in the ULS extension uses the API already. Why is this overridden by Translate? Is this used in production?

It's not code, but most times I've seen setlang used (which aren't that many to be fair), it has been from Wikimedia projects linking to especially Wikimedia Commons, to let visitors to Commons use the same interface language of the Wikipedia they arrive to Commons from. Is there any way of finding all of those types of links as well? (Short of searching every single wiki…)

One (with access) could run mwgrep to find those. It's probably too much for us to fix ourselves (if we even could, restricted editing etc.) so I think we want to rely on soft deprecation and have other people fix the majority first.

Change 551940 had a related patch set uploaded (by Abijeet Patro; owner: Abijeet Patro):
[mediawiki/extensions/UniversalLanguageSelector@master] [WIP] Handle setlang

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

The functionality associated with the setlang query parameter has now been updated. If the setlang parameter is present in the query string, we display a dialog to the user to confirm whether they want to update their preferred interface language.

image.png (680×990 px, 43 KB)

Thanks @Pginer-WMF for help with dialog design.

There are a few cases here -

  1. Logged in user with setlang parameter set to a language code different then the user's current preferred interface language. In this case, we display the confirmation dialog.
  2. Logged in user with setlang parameter set to a language code that is the same as the user's current preferred interface language. In this case, we do not display the confirmation dialog.
  3. Anonymous user with setlang parameter passed and $wgULSAnonCanChangeLanguage set to true, plus the above two scenarios.
  4. Anonymous user with setlang parameter passed but with $wgULSAnonCanChangeLanguage set to false. In this case, we do not display the confirmation dialog.

In addition, a check is done to ensure that the language code passed via setlang is valid, otherwise no dialog is shown.

The URL parameter setlang will no longer immediately change the user interface language. When a user opens a such link, they will be shown a language change confirmation dialog.

The dialog is not be shown if JavaScript is not available.

Change 551940 merged by jenkins-bot:
[mediawiki/extensions/UniversalLanguageSelector@master] Update setlang to display confirmation dialog to change language

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

abi_ added a subscriber: Jpita.

Thanks @Fomafix for your inputs on the patch and the subsequent fixes that you did.

@Jpita - We can use the MLEB instance for testing - https://language-translate.wmflabs.org/index.php/Main_Page this patch further. I'll send you the credentials over PM.

Following are the test cases,

  1. Logged in user with setlang parameter set to a language code different then the user's current preferred interface language. In this case, we display the confirmation dialog.
  2. Logged in user with setlang parameter set to a language code that is the same as the user's current preferred interface language. In this case, we do not display the confirmation dialog.
  3. Anonymous user with setlang parameter passed and $wgULSAnonCanChangeLanguage set to true, plus the above two scenarios. After accepting the change, the anonymous user should still be able to update his language by using ULS on the front end.
  4. Anonymous user with setlang parameter passed but with $wgULSAnonCanChangeLanguage set to false. In this case, we do not display the confirmation dialog.

Irrespective of whether or not the user accepts the change via the dialog, the setlang parameter should be removed from the URL.

  • The dialog doesn't have a support for keyboard navigation.
  • A click on Don't change removes the URL parameter setlang. But ...
    • ... on some links like Printable version, Login, Mobile view the URL parameter is still there and on a click the dialog comes again.
    • ... on history back the URL parameter setlang is there again. (Fixed by https://gerrit.wikimedia.org/r/559573)
  • When the value of the URL parameter setlang is equal to the current user interface language then there is no dialog but the URL parameter setlang gets not removed. On a language change with the ULS language changer the new page is loaded with the old setlang value and the dialog wants to switch back to the old language. I think it is better to remove the URL parameter setlang in this case. (Fixed by https://gerrit.wikimedia.org/r/559575)
  • The scroll position is wrong when there is an URL with a fragment like Special:Version?setlang=de#Installed_software (Fixed by https://gerrit.wikimedia.org/r/559775)
  • The dialog doesn't have a support for keyboard navigation.

Aye, yeah, using Tab goes through the entire page header body and footer before reaching the dialog. And then, it starts with the button, not the header. We'll want to start by programmatically moving the focus to the content of the dialog or its first child somehow.

  • The dialog doesn't have a support for keyboard navigation.

Aye, yeah, using Tab goes through the entire page header body and footer before reaching the dialog. And then, it starts with the button, not the header. We'll want to start by programmatically moving the focus to the content of the dialog or its first child somehow.

Submitted a patch for this - https://gerrit.wikimedia.org/r/559795

IIUC, after the change setlang would change the preferred interface language of a user permanently when they accept the change. In that case I think the following string communicates that very well.

The link you followed requested the interface to be shown in $lang

On reading its possible for users to think that it would only change the language temporarily.

Change 559795 had a related patch set uploaded (by Abijeet Patro; owner: Abijeet Patro):
[mediawiki/extensions/UniversalLanguageSelector@master] Add support for keyboard events for the setlang dialog

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

@KartikMistry can u deploy this on language-translate.wmflabs.org so I can test it please?

@KartikMistry can u deploy this on language-translate.wmflabs.org so I can test it please?

Lets wait till Niklas has had a chance to review and merge that patch.

Change 564559 had a related patch set uploaded (by Abijeet Patro; owner: Abijeet Patro):
[mediawiki/extensions/UniversalLanguageSelector@master] Remove setlang URL parameter on dialog close

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

Change 559795 merged by jenkins-bot:
[mediawiki/extensions/UniversalLanguageSelector@master] Add support for keyboard events for the setlang dialog

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

@abi_ is this now deployed on language-translate.wmflabs?

@Jpita - Yes, it has been, but Niklas subsequently found another issue (Closing the dialog via Esc or clicking on the overlay was not removing the setlang parameter from the URL) which was fixed in a separate patch - https://gerrit.wikimedia.org/r/564559. Lets wait for that to get merged and deployed too.

Change 564559 merged by jenkins-bot:
[mediawiki/extensions/UniversalLanguageSelector@master] Remove setlang URL parameter on dialog close

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

@Jpita - This is now deployed on language-translate.wmflabs and can be tested.

This is done. Tested on MediaWiki.

To summarize, when the setlang parameter is present in the URL, instead of changing the language immediately, we're now displaying a dialog box to ask the user for confirmation.

image.png (680×990 px, 43 KB)

Thank you for doing this. Greatly appreciated