From d3869e581e94bdc6b9eb6038b1a351b211f0ae49 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerg=C5=91=20Tisza?= <tgr.huwiki@gmail.com>
Date: Sat, 23 Aug 2014 17:14:37 +0000
Subject: [PATCH] Fix preference DB values

Ensure that changing the preference via the quick link and
Special:Preferences is stored identically in the DB.

Change-Id: Ia37da1c6bfbb3edf0eba56f01105e4a5f3a5e4ba
---
 MultimediaViewerHooks.php          |  4 ++++
 resources/mmv/mmv.Config.js        | 19 ++++++++++++++++---
 tests/qunit/mmv/mmv.Config.test.js |  2 +-
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/MultimediaViewerHooks.php b/MultimediaViewerHooks.php
index 6ac204aaa..9d40dee4f 100644
--- a/MultimediaViewerHooks.php
+++ b/MultimediaViewerHooks.php
@@ -164,8 +164,12 @@ class MultimediaViewerHooks {
 	 * @param OutputPage $out
 	 */
 	public static function makeGlobalVariablesScript( &$vars, OutputPage $out ) {
+		global $wgDefaultUserOptions;
+
 		$user = $out->getUser();
 		$vars['wgMediaViewerOnClick'] = self::shouldHandleClicks( $user );
+		// needed because of bug 69942; could be different for anon and logged-in
+		$vars['wgMediaViewerEnabledByDefault'] = !empty( $wgDefaultUserOptions['multimediaviewer-enable'] );
 	}
 
 	/**
diff --git a/resources/mmv/mmv.Config.js b/resources/mmv/mmv.Config.js
index 341cdebe8..e7f47db6d 100644
--- a/resources/mmv/mmv.Config.js
+++ b/resources/mmv/mmv.Config.js
@@ -140,7 +140,9 @@
 	 * @return {jQuery.Promise} a deferred which resolves/rejects on success/failure respectively
 	 */
 	CP.setMediaViewerEnabledOnClick = function ( enabled ) {
-		var config = this,
+		var newPrefValue,
+			defaultPrefValue = this.mwConfig.get( 'wgMediaViewerEnabledByDefault' ),
+			config = this,
 			success = true;
 
 		if ( this.mwUser.isAnon() ) {
@@ -150,14 +152,25 @@
 				success = this.removeFromLocalStorage( 'wgMediaViewerOnClick' );
 			}
 			if ( success ) {
+				// make the change work without a reload
 				config.mwConfig.set( 'wgMediaViewerOnClick', enabled );
 				return $.Deferred().resolve();
 			} else {
 				return $.Deferred().reject();
 			}
 		} else {
-			return this.setUserPreference( 'multimediaviewer-enable', enabled ? true : '').then( function () {  // wow our prefs API sucks
-				// make the change work without a reload
+			// Simulate changing the option in Special:Preferences. Turns out this is quite hard (bug 69942):
+			// we need to delete the user_properties row if the new setting is the same as the default,
+			// otherwise set '1' for enabled, '' for disabled. In theory the pref API will delete the row
+			// if the new value equals the default, but this does not always work.
+			if ( defaultPrefValue === true ) {
+				newPrefValue = enabled ? '1' : '';
+			} else {
+				// undefined will cause the API call to omit the optionvalue parameter
+				// which in turn will cause the options API to delete the row and revert the pref to default
+				newPrefValue = enabled ? '1' : undefined;
+			}
+			return this.setUserPreference( 'multimediaviewer-enable', newPrefValue ).then( function () {
 				config.mwConfig.set( 'wgMediaViewerOnClick', enabled );
 			} );
 		}
diff --git a/tests/qunit/mmv/mmv.Config.test.js b/tests/qunit/mmv/mmv.Config.test.js
index 8f0d2261f..f1214d91e 100644
--- a/tests/qunit/mmv/mmv.Config.test.js
+++ b/tests/qunit/mmv/mmv.Config.test.js
@@ -136,7 +136,7 @@
 	QUnit.test( 'setMediaViewerEnabledOnClick sanity check', 3, function ( assert ) {
 		var localStorage = { setItem: this.sandbox.stub(), removeItem: this.sandbox.stub() },
 			mwUser = { isAnon: this.sandbox.stub() },
-			mwConfig = { set: this.sandbox.stub() },
+			mwConfig = new mw.Map( { wgMediaViewerEnabledByDefault: false } ),
 			api = { postWithToken: this.sandbox.stub() },
 			config = new mw.mmv.Config( {}, mwConfig, mwUser, api, localStorage );