Page MenuHomePhabricator

Sort mobile web language switcher list case insensitive
Closed, ResolvedPublic3 Estimated Story Points

Description

Presently, the mobile web language switcher sorts languages alphabetically, but does so with case sensitivity.

During user testing this was identified as confusing.

Languages should be sorted in a case insensitive fashion. The general outcome will be that vastly different character sets (e.g., Latinized versus many Eastern languages) will continue to be in the same general part of the list, but within a given script (e.g. Latinized characters) the first letter character casing won't result in stuff like "bosanski" being listed after "Deutsch" (in the mock bosanski would be the 3rd language in the list).

Acceptance Criteria

  • Languages are sorted alphabetically, but in a case insensitive fashion
  • Event logging should continue to identify language positioning correctly
  • This sort should be updated for both JavaScript and non-JavaScript version
  • PHPUnit test should be written for SpecialMobileLanguages

Event Timeline

dr0ptp4kt created this task.Jul 8 2016, 3:41 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJul 8 2016, 3:41 PM
dr0ptp4kt updated the task description. (Show Details)Jul 11 2016, 4:40 PM
Jdlrobson updated the task description. (Show Details)Jul 11 2016, 4:43 PM
Jdlrobson set the point value for this task to 3.Jul 11 2016, 4:48 PM
Jdlrobson lowered the priority of this task from High to Medium.Jul 11 2016, 5:15 PM
Jdlrobson moved this task from Incoming to 2016-17 Q2 on the Readers-Web-Backlog board.
jhobs moved this task from To Do to Doing on the Reading-Web-Sprint-77-Segmentation-fault board.

Change 300438 had a related patch set uploaded (by Jhobs):
Sort languages under "All Languages" case-insensitive

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

Still needs more work because I'm writing the phpunit test, but moved back to Code Review column for visibility of new patch set that addressed comments.

jhobs updated the task description. (Show Details)Jul 26 2016, 4:31 PM

The patch is fine. Waiting on the PHPUnit test now so we can verify it does what it should and merge.

Change 301649 had a related patch set uploaded (by Jhobs):
Sort languages displayed on Special:MobileLanguages

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

After @Jdlrobson and I paired, we decided the original patch should be split into 3: one containing the frontend changes, one containing a test for the existing behavior of Special:MobileLanguages, and one containing the backend changes (which would also update the test). After some consideration, I decided the second patch was unnecessary as it was minimally different from the third and caused unnecessary dependency complexity, so I just split the patches into two.

I'm doing a bit of hygiene on the second patch, but this task is ready for review.

@hashar Please see https://gerrit.wikimedia.org/r/301649, it seems our Jenkins slaves may be misconfigured or something. It's returning the desktop domain when mobile domain is specifically requested during PHPUnit tests.

Just for info: I do not scale anymore and can not take care of all craziness happening, I am also going to be on vacations soonish.

I have no idea how: MobileContext::singleton()->getMobileUrl( $langObject['url'] ) works at detecting whether it is on mobile or not. Seems $wgMobileUrlTemplate is not set to what you expect, maybe add a test that assert it is properly you could them make it a dependency of the testProcessLanguages. Something like:

function testWgMobileUrlTemplateIsProper() {
    global $wgMobileUrlTemplate;
    $this->assertEquals( 'whatever you want', $wgMobileUrlTemplate,
        '$wgMobileUrlTemplate is improperly set'
    );
}

/***
 * @depends testWgMobileUrlTemplateIsProper
 */
function testProcessLanguages...

The testProcessLanguages using a dataprovider will be skipped entirely if the first test fail.

Might want to do the same with $langObject['url'].

A note: CI runs the tests with debug log enabled which are captured and made available on the build page. https://integration.wikimedia.org/ci/job/mwext-testextension-hhvm/19209/ look for the build artifact.

Change 300438 merged by jenkins-bot:
Sort languages under "All Languages" case-insensitive

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

Minor fix needed. Will merge tomorrow provided you can get that done by lunchtime.

@hashar sorry for wasting your time here. @jhobs usually Jenkins is pretty accurate so always best to troubleshoot with team first.

@hashar sorry for wasting your time here. @jhobs usually Jenkins is pretty accurate so always best to troubleshoot with team first.

I am more than happy to help in diagnostic of tests failures! There is always a possibility that the fault is on Jenkins / CI or mediawiki test runner, so that has to be ruled out :]

I merely pointed that adding a project will reach out more people than just me. Specially when I am going to be on vacations :D Happy you apparently got it fixed or at least figured out \O/

dr0ptp4kt closed this task as Resolved.Jul 29 2016, 8:29 PM

Signing off.