Page MenuHomePhabricator

[Bug] All mobile options reverted when desktop preferences saved
Closed, ResolvedPublicBUG REPORT

Description

Steps to reproduce

  1. Login and visit https://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=Special:MobileOptions on the MinervaNeue mobile site.
  2. Enable AMC.
  3. Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Special:Preferences.
  4. Toggle "Default to newcomer homepage from username link in personal tools" and save.

Expected results

  • Special:MobileOptions persist.

Actual results

  • All Special:MobileOptions are reverted.

Environments observed

  • Browser version: Chromium v75.0.3770.90
  • OS version: Ubuntu v19.04
  • Device model: Desktop
  • Device language: English

Questions

  • Does this affect the AMC opt-in and opt-out rates?

Developer notes

The problem is that the amc user preference registration is done incorrectly. It uses only UserGetDefaultOptions hook to set a default user option value, but it does not use register option via GetPreferences hook.
Most probably GetPreferences hook wasn't handled because we didn't want to show AMC mode on Special:Preferences page. But we need to register it with type=api so option is registered, but not visible on the Special:Preferences page.

What currently happens:
Everything starts in DefaultPreferencesFactory::saveFormData()

  • DefaultPreferencesFactory::saveFormData() calls user->resetOptions( 'unused', $form->getContext() );
  • User::resetOptions loads all user options and uses options definitions (loaded via GetPreferences hook)
  • if option definition does not exist it's marked as 'unused' and later removed


Important note: AMC is not the only option that is removed on the Special:Preferences save. MediaWiki core also removes:

  • mobile-specialpages
  • mfMode -- Beta Mode works only because we set up the cookie. The beta-opt-out graph might be broken.

Please fix also those.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 25 2019, 2:02 PM
Niedzielski updated the task description. (Show Details)
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
Jdlrobson reassigned this task from Jdlrobson to pmiazga.Jul 25 2019, 6:04 PM
Jdlrobson added a subscriber: Jdlrobson.

Needs some analysis from Piotr and then an estimation.

Change 526277 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/MobileFrontend@master] Register mfMode and mf_amc_optin via GetPreferences hook

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

Jdlrobson triaged this task as Medium priority.Jul 29 2019, 11:27 PM

Change 526277 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Register mfMode and mf_amc_optin via GetPreferences hook

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

Jdlrobson reassigned this task from pmiazga to Niedzielski.Jul 30 2019, 12:41 AM

Looks fixed to me. @Niedzielski can you confirm and resolve?

Niedzielski closed this task as Resolved.Jul 30 2019, 1:30 PM

LGTM