Page MenuHomePhabricator

[Compact links] Don't show as a beta feature option if it can't be enabled
Closed, ResolvedPublic

Description

  1. Visit https://test.wikipedia.org/wiki/Main_Page
  2. Go to https://test.wikipedia.org/wiki/Special:Preferences#mw-prefsection-betafeatures
  3. Notice and check the option for "Compact language links"
  4. Go back to the main page

I. Observed: Nothing changed.
II. Expected: I'm not offered the option if it doesn't have an effect.

Introduced by https://gerrit.wikimedia.org/r/#/c/117212/ , see also bug 62361.


Version: master
Severity: normal

Details

Reference
bz62362

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:07 AM
bzimport set Reference to bz62362.

https://gerrit.wikimedia.org/r/#/c/117366/ was approved and backported, but I still see the incorrect behaviour.

To clarify: the beta feature is offered, but the feature does nothing because of the differing checks for the preference and the actual feature.

Preference:
+ if ( $wgULSCompactLinks ) {
+ $prefs['uls-compact-links'] = array(

Feature:
+ if ( $wgULSCompactLinks &&
+ $wgULSPosition === 'interlanguage' &&
+ class_exists( 'BetaFeatures' ) &&

        BetaFeatures::isFeatureEnabled( $out->getUser(), 'uls-compact-links' )
) {
        $out->addModules( 'ext.uls.compactlinks' );

(Test Wikipedia is one of those wikis which has ULS in personal position but also has interwiki links)

Change 117394 had a related patch set uploaded by Niharika29:
Don't show compact-link beta feature in list if it cannot be enabled

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

The beta feature should not be listed on those wikis where activating it has no effect.

Currently the ULS position is used as a conservative way to make sure the feature only applies to wikis where we are sure interlanguage links exist.
We can also consider to base this logic on $wgInterwikiMagic $wgHideInterlanguageLinks instead to both list and activate the feature only on wikis with interlanguage links support.

Change 117394 abandoned by Niharika29:
Don't show compact-link beta feature in list if it cannot be enabled

Reason:
Not needed.

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

(In reply to Gerrit Notification Bot from comment #5)

Change 117394 abandoned by Niharika29:
Don't show compact-link beta feature in list if it cannot be enabled

Reason:
Not needed.

But the checks are still differing between preference and feature, as Niklas said.

Checks to show the feature in BFs list:
$wgULSCompactLinks && $wgInterwikiMagic == true && $wgHideInterlanguageLinks == false
This will ensure the feature is available on wikis which have interlanguage links irrespective of ULS position IFF $wgULSCompactLinks is true.

Checks to load the module:
$wgULSCompactLinks && class_exists( 'BetaFeatures' ) &&
BetaFeatures::isFeatureEnabled( $out->getUser(), 'uls-compact-links' )

We check the global flag in addition to checking whether the user has opted for the beta feature or not. This is important because if we don't check it here and the flag is turned false at some point of time, then the users will be stuck with the feature ON and will have no way to turn it OFF.

I hope that clears it up.

(In reply to Niklas Laxström from comment #2)

To clarify: the beta feature is offered, but the feature does nothing
because of the differing checks for the preference and the actual feature.

Now it is: A) adding the preference

		if ( $wgULSCompactLinks &&
			$wgInterwikiMagic === true &&
			$wgHideInterlanguageLinks === false
		) { ...

B) actually adding the feature

		if ( $wgULSCompactLinks &&
			class_exists( 'BetaFeatures' ) &&
			BetaFeatures::isFeatureEnabled( $out->getUser(), 'uls-compact-links' )
		) { ...

B can't be false unless A is also false (or invalid), but syncing the two would make the thing cleaner.

Change 132819 had a related patch set uploaded by Nemo bis:
Compact links beta feature conditions sync

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

Change 132819 merged by jenkins-bot:
Compact links beta feature conditions sync

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