Page MenuHomePhabricator

Respect user specified languages in CSR
Closed, ResolvedPublic3 Story Points

Description

wb getUserLanguages leverages mw.config.get( 'wbUserSpecifiedLanguages' ) and falls back to mw.uls.getFrequentLanguageList() while termbox only makes use of the latter in CSR. In order to provide consistent and correctly user-tailored results in termbox we should change the implementation accordingly.

AC

  • termbox CSR uses mw.config.get( 'wbUserSpecifiedLanguages' ) and falls back to mw.uls.getFrequentLanguageList() to determine user's preferred languages

Info

  • termbox SSR already receives the (correct) result of LanguageFallbackChainFactory::newFromContext( OutputPage ) but CSR overwrites this with incorrectly tailored preferred languages when it comes into effect

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 12 2019, 3:19 PM
Pablo-WMDE updated the task description. (Show Details)
Pablo-WMDE updated the task description. (Show Details)Mar 13 2019, 9:21 AM
Pablo-WMDE updated the task description. (Show Details)Mar 13 2019, 9:29 AM
Lea_WMDE set the point value for this task to 3.Mar 13 2019, 3:54 PM

Change 496218 had a related patch set uploaded (by Jakob; owner: Jakob):
[mediawiki/extensions/Wikibase@master] Extract getUserLanguages from entityViewInit for reuse

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

Pablo-WMDE added a comment.EditedMar 13 2019, 6:43 PM

@Jakob_WMDE Thanks for picking this up. It appears T213732 first mentions the magic number 4 (now USER_TOP_SECONDARY_LANGUAGES_LIMIT) but does so by incompletely quoting entityViewInit's getUserLanguages(). If we now directly consume getUserLanguages() (which already ensures this limit for uls.getFrequentLanguageList(), and only for this) it looks like we can completely lose our custom limit.
I could not find a product requirement stating "termbox should never show more than 4 languages in the more languages section", rather "the 'more languages' section contains entity information in the languages that are most likely to be spoken by the user, minus the primary language already shown above" (T211865).

Change 496442 had a related patch set uploaded (by Matthias Geisler; owner: Matthias Geisler):
[wikibase/termbox@master] Switch to wb.getUserLanguages()

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

Change 496779 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] getUserLanguages: polish tests of newly extracted method

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

Change 496780 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] getUserLanguages: allow use in termbox

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

Jakob_WMDE moved this task from Peer Review to Doing on the Wikidata-Termbox-Iteration-11 board.

Change 496442 had a related patch set uploaded (by Jakob; owner: Matthias Geisler):
[wikibase/termbox@master] Switch to wb.getUserLanguages()

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

Change 496218 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Extract getUserLanguages from entityViewInit for reuse

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

Change 496779 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] getUserLanguages: polish tests of newly extracted method

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

Change 496780 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] getUserLanguages: allow use in termbox

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

Change 496442 merged by jenkins-bot:
[wikibase/termbox@master] Switch to wb.getUserLanguages()

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

Change 497774 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] termbox: update pin

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

This seems to me to be done in wikibase/termbox but we are still waiting on updating the pin. This is blocked by diffusion / gerrit sadness. See: T218780

Change 497774 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] termbox: update pin

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

I'm not sure how I can test the CRS part of it. I did see the beautiful babel box magic in a ticket I was testing earlier this week, but I understand it right that this ticket is specifically about having it rendered in CSR, not SSR? If yet, how can I produce such a situation?

Pablo-WMDE closed this task as Resolved.Apr 8 2019, 10:39 AM