Page MenuHomePhabricator

Disable Media Viewer by default for logged-in users on Wikimedia Commons
Closed, ResolvedPublic

Description

Per the WMF response to the Media Viewer RfC on Commons:

https://commons.wikimedia.org/wiki/Commons_talk:Requests_for_comment/Media_Viewer_software_feature#Response_to_the_Media_Viewer_RfC_on_Wikimedia_Commons

Media Viewer should be enabled by default for anonymous users, but disabled for logged-in users.


Version: wmf-deployment
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=69080

Details

Reference
bz69363

Event Timeline

bzimport raised the priority of this task from to Normal.
bzimport set Reference to bz69363.
bzimport added a subscriber: Unknown Object (MLST).
Tgr created this task.Aug 10 2014, 10:52 AM

gerritadmin wrote:

Change 154395 had a related patch set uploaded by Gergő Tisza:
Add an option to disable by default for logged-in users

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

gerritadmin wrote:

Change 154396 had a related patch set uploaded by Gergő Tisza:
Disable MediaViewer by default for logged-in users on Commons

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

gerritadmin wrote:

Change 154396 merged by jenkins-bot:
Disable MediaViewer by default for logged-in users on Commons

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

gerritadmin wrote:

Change 155443 had a related patch set uploaded by MarkTraceur:
Add an option to disable by default for logged-in users

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

gerritadmin wrote:

Change 154395 merged by jenkins-bot:
Add an option to disable by default for logged-in users

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

gerritadmin wrote:

Change 155443 merged by MarkTraceur:
Add an option to disable by default for logged-in users

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

Note: Also ran a query manually to set the opt-in preference if the user disabled the gadget. It was like 50 people.

Tgr added a comment.Sep 3 2014, 2:24 PM

The change of default caused some junk to be written in the user_properties table (details in bug 69942):

select up_value, count(*) from user_properties where up_property = 'multimediaviewer-enable' group by up_value;

+----------+----------+

up_valuecount(*)

+----------+----------+

1560
02
197
true17

+----------+----------+

Fixed in https://gerrit.wikimedia.org/r/#/c/156004/ , needs DB cleanup.

Tgr added a comment.Sep 3 2014, 2:33 PM

The 'toggle' preference type sets an up_value of '1' for enabled, '' for disabled, but the row is omitted if the setting is the same as the default. Commons has MediaViewer disabled as default for logged-in users, so the cleanup query is

UPDATE user_properties SET up_value = '1' WHERE up_property = 'multimediaviewer-enable' AND up_value = 'true';
DELETE FROM user_properties WHERE up_property = 'multimediaviewer-enable' AND up_value IN ( '', '0' );

(In reply to Tisza Gergő from comment #8)

Fixed in https://gerrit.wikimedia.org/r/#/c/156004/ , needs DB cleanup.

Tisza: Who's going to do the DB cleanup (and when)?

Tgr added a comment.Sep 8 2014, 3:30 PM

(In reply to Andre Klapper from comment #10)

Tisza: Who's going to do the DB cleanup (and when)?

Someone from the Multimedia team will do this. Probably this week, although it is not particularly urgent (functionally there is no difference between the correct and incorrect values, but it's ugly and will probably mess up stats scripts).

Query reviewed. You can go ahead and run it, Gergo.

Tgr added a comment.Oct 1 2014, 8:38 PM

tgr@terbium:~$ mwscript sql.php --wiki=commonswiki clean-mmv-user-property.sql
Query OK, 17 row(s) affected
Query OK, 1555 row(s) affected

...

mysql:research@s4-analytics-slave.eqiad.wmnet [commonswiki]> select up_value, count(*) from user_properties where up_property = 'multimediaviewer-enable' group by up_value;
+----------+----------+

up_valuecount(*)

+----------+----------+

3
1223

+----------+----------+
2 rows in set (0.01 sec)

So, um. MV still seems to be setting '' values. Probably I should redo the optout patch properly, with $wgDefaultUserOptions['multimediaviewer-enable'] set to 1 instead of true. I'll do that once I don't have anything important to do.

What is the next step to perform here, and by who?

Tgr added a comment.Oct 27 2014, 1:56 PM

Since this is not really related to the original request anymore, I moved it to bug 72557.

Gilles removed a subscriber: Gilles.Apr 23 2015, 7:46 PM