[go: up one dir, main page]

Page MenuHomePhabricator

[S] Make Special:Contribute the default entry point in the menu
Closed, ResolvedPublic1 Estimated Story Points

Description

Currently, there is a link to Special:Contributions in the user menu. In T319240, we will conditionally show tabs inside the Special:Contribute page.

Using the $wgSpecialContributeSkinsDisabled configuration flag we have added in T319240 should also be applied to the menu item, so that the default experience is to navigate to the contributions entry point page defined here:
https://github.com/wikimedia/mediawiki/blob/6fbe889e2694c94edaeb9906111bb20aa465eef4/includes/skins/SkinTemplate.php#L471

TODO

  • When page is active the label should be Contribute
  • When page is active the icon should be a pencil not userContributions

Related Objects

StatusSubtypeAssignedTask
OpenNone
OpenNone
OpenNone
OpenNone
ResolvedJdlrobson
ResolvedTgr
DuplicateNone
Resolvedovasileva
ResolvedJdlrobson
ResolvedJdlrobson
ResolvedJdlrobson
ResolvedNone
ResolvedJdlrobson
ResolvedJdlrobson
ResolvedAmmarpad
Resolvedovasileva
Resolved Mabualruz
Resolvedovasileva
Resolvedovasileva
ResolvedKartikMistry

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: Mabualruz.

Given the approach @Mabualruz has taken on https://phabricator.wikimedia.org/T319240 we might as well do this now while it's fresh in our mind.

Change 838733 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/core@master] Make Special:Contribute the default entry point in the menu - Attention: Pencil icon is still not showing in minerva skin

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

LGoto set the point value for this task to 1.Oct 5 2022, 4:32 PM
LGoto renamed this task from Make Special:Contribute the default entry point in the menu to [S] Make Special:Contribute the default entry point in the menu.Oct 5 2022, 4:54 PM

Change 838733 merged by Mabualruz:

[mediawiki/core@master] Skins: Config flag controls contributions link

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

Change 841162 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/core@master] Revert "Revert "Skins: Config flag controls contributions link""

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

Change 841480 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ContentTranslation@master] AddContributeCardEntryPoint: Use RequestContext::getMain

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

Change 841480 merged by jenkins-bot:

[mediawiki/extensions/ContentTranslation@master] AddContributeCardEntryPoint: Use RequestContext::getMain

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

Change 841509 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/ContentTranslation@wmf/1.40.0-wmf.5] AddContributeCardEntryPoint: Use RequestContext::getMain

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

Change 841509 merged by jenkins-bot:

[mediawiki/extensions/ContentTranslation@wmf/1.40.0-wmf.5] AddContributeCardEntryPoint: Use RequestContext::getMain

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

Change 841872 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/ContentTranslation@wmf/1.40.0-wmf.5] Revert "AddContributeCardEntryPoint: Use RequestContext::getMain"

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

Change 841872 merged by Urbanecm:

[mediawiki/extensions/ContentTranslation@wmf/1.40.0-wmf.5] Revert "AddContributeCardEntryPoint: Use RequestContext::getMain"

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

Hello @xxx, you merged https://gerrit.wikimedia.org/r/841509, but you did not deploy the commit to production (see https://wikitech.wikimedia.org/wiki/How_to_deploy_code). This is a problem, because it confuses other deployers: the wmf branches should represent the status in production as-is. See Problem: Undeployed code for more details.

If you want to schedule the backport for deployment, please upload it again and schedule it at one of the backport windows in the deployments calendar. Otherwise, it will be deployed with next train (tomorrow, if everything goes according to the plan).

If you have any questions, please let me know!

Using the $wgSpecialContributeSkinsDisabled configuration flag we have added in T319240 should also be applied to the menu item, so that the default experience is to navigate to the contributions entry point page defined here:

I think this flag needs to be "flipped" to $wgSpecialContributeSkinsEnabled.

Currently the default value lists all skins used on Wikimedia wikis:

	public const SpecialContributeSkinsDisabled = [
		'default' => [ 'vector', 'vector-2022', 'monobook',
			'timeless', 'modern', 'cologneblue', 'minerva' ],

However, this doesn't consider third-party skins – the feature would be enabled by default on all of them, until each site administrator changes their configuration.

If we used SpecialContributeSkinsEnabled instead, defaulting to [], this would work better out-of-the-box. Our own configuration in operations/mediawiki-config would be simplified too (to enable on Minerva, you'd just add Minerva to the list, instead of having to write 6 skins).

Change 851132 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[operations/mediawiki-config@master] Update wgSpecialContributeSkinsDisabled → wgSpecialContributeSkinsEnabled

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

I think this flag needs to be "flipped" to $wgSpecialContributeSkinsEnabled.
However, this doesn't consider third-party skins – the feature would be enabled by default on all of them, until each site administrator changes their configuration.

FWIW The rationale for making it $wgSpecialContributeSkinsDisabled was that this is more developer/3rd party friendly. :-) We used the allow list approach in Extension:RelatedArticles and it trips up a lot of 3rd parties as wgRelatedArticlesFooterAllowedSkins needs to be configured each time you add a new skin. We want 3rd parties to use this and give us feedback!

The only reason all skins is disabled right now is because the language team is not ready with their features and focusing primarily on the mobile experiences. Shipping this to desktop skins would likely involve additional community outreach but I believe the eventual goal is to have this feature on as many skins as possible and get rid of the feature flag altogether.

FWIW The rationale for making it $wgSpecialContributeSkinsDisabled was that this is more developer/3rd party friendly. :-) We used the allow list approach in Extension:RelatedArticles and it trips up a lot of 3rd parties as wgRelatedArticlesFooterAllowedSkins needs to be configured each time you add a new skin. We want 3rd parties to use this and give us feedback!

The only reason all skins is disabled right now is because the language team is not ready with their features and focusing primarily on the mobile experiences. Shipping this to desktop skins would likely involve additional community outreach but I believe the eventual goal is to have this feature on as many skins as possible and get rid of the feature flag altogether.

I agree in principle, but in practice, the feature does not work on skins other than Vector-2022 and Minerva (because it requires the skin to support special pages' navigation links [T313349] to keep the link to Special:Contributions accessible). It may be good for third-party developers to expose that, but it seems bad for third-party site administrators / users.

If the config setting was intended for skin developers and not site administrators, then I'd consider not making it a config setting at all – it could be a skin attribute defined in skin.json, which would allow skins to opt-in once they add support for T313349 (without requiring site administrators to update their config).

If the config setting was intended for skin developers and not site administrators, then I'd consider not making it a config setting at all

The configuration was meant to be temporary. Skin developers opt into it my including "associated-pages" when they declare the menus that they use e.g. here:
https://github.com/wikimedia/Vector/blob/master/skin.json#L53

Pages can then check whether a skin supports a certain menu by using Skin::supportsMenu like in this patch:

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/813711/13/includes/specials/SpecialEditWatchlist.php#181

If the menu is not supported SpecialContributions can render the menu as a subtitle, or add to a different menu, for example.

Thanks. That solution makes a lot of sense. But:

  • How was I, or anyone else not on your team, supposed to know that the config setting is temporary?
  • What is the purpose of the config setting if you already have a better way of achieving the same thing?
  • Who and when is going to work on that? If the config setting is going to make it into a MediaWiki release, then it needs to work correctly.

I think we can support both for now developer friendly + skin config and we can remove the developer approach once it is used in the skins designated for it

As a skin developer, I agree with @matmarex here that making it a config would make it less friendly to third parties, at the very least for skin developers. The menu registration from @Jdlrobson is a much better approach.

FWIW The rationale for making it $wgSpecialContributeSkinsDisabled was that this is more developer/3rd party friendly. :-) We used the allow list approach in Extension:RelatedArticles and it trips up a lot of 3rd parties as wgRelatedArticlesFooterAllowedSkins needs to be configured each time you add a new skin. We want 3rd parties to use this and give us feedback!

While I don't fully understand the rationale behind these decisions from the product teams, the config flag approach would make it less developer-friendly because:

  • Site admins are the people who actually need to deal with these configs, often time they are not equipped with the knowledge to do so. But assuming that skin developers take the role of communicating that:
  • They are specific to the extension. Skin developers need to have explicit knowledge about the extension.
  • They (especially temporary ones) are difficult to keep up with. Skin developers need to stay updated with the development of the extension. It becomes more complicated when the skin release policy does not match with the extension's (LTS vs WMF release policy).
  • They require more communication and outreach. Skin developers need to be able to find that information and extension developers need to communicate it, it is just more work for both sides assuming that there are communications at all.
  • They are making assumptions that skins can not support the extension, and skins have no way to control that. That should be decided by skin developers, as it is the skin's responsibility to style user interface at the end. (e.g. T257746)

Tracking down all these information require a lot of investigation skills and it only makes skins even more difficult to develop as extensions is a part of the cohesive experience. As a reference, I have been developing a skin that follows LTS policy. To find these changes, I have to read through the commits on both the specific extension (e.g. VisualEditor) and the supported skin (e.g. Vector 2022) from latest to last LTS, find the Phabricator task from the commit, and read the task and all the other connected tasks and patches. It is a lot of work to be expected of third parties, reducing the chances of getting feedbacks.

The only reason all skins is disabled right now is because the language team is not ready with their features and focusing primarily on the mobile experiences. Shipping this to desktop skins would likely involve additional community outreach but I believe the eventual goal is to have this feature on as many skins as possible and get rid of the feature flag altogether.

In what way is it not ready for a desktop skin?

Now we support both approaches on the patch, for quick developer check with the config, and more stable for wider use now also adding it to a skin's skin.json with 'special-contribute' menu name under "menus".

I've approved the patch – it's still confusing to me why we have several ways to enable the new link, but that code was already there before I was asked to review the work on this task, so I don't think it's fair to block this now. Also, with the latest changes in the patch (flipping the config setting), if you (as a skin developer or a site administrator) ignore this completely, then at least nothing will break for you.

I hope we can work out a simpler way to control whether the new link is shown before the 1.40 release.

Change 841162 merged by jenkins-bot:

[mediawiki/core@master] Skins: Config flag controls contributions link

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

Change 851132 merged by jenkins-bot:

[operations/mediawiki-config@master] Update wgSpecialContributeSkinsDisabled → wgSpecialContributeSkinsEnabled

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

Mentioned in SAL (#wikimedia-operations) [2022-11-08T21:08:27Z] <urbanecm@deploy1002> Started scap: Backport for [[gerrit:833831|Enable history page visual diffs on beta cluster (T314588)]], [[gerrit:851132|Update wgSpecialContributeSkinsDisabled → wgSpecialContributeSkinsEnabled (T319327)]]

Mentioned in SAL (#wikimedia-operations) [2022-11-08T21:13:00Z] <urbanecm@deploy1002> Finished scap: Backport for [[gerrit:833831|Enable history page visual diffs on beta cluster (T314588)]], [[gerrit:851132|Update wgSpecialContributeSkinsDisabled → wgSpecialContributeSkinsEnabled (T319327)]] (duration: 04m 33s)

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: Currently, there is a link to Special:Contributions in the user menu. In T319240, we will conditionally show tabs inside the Special:Contribute page.

Screenshot 2022-11-09 at 7.03.11 PM.png (535×1 px, 74 KB)

✅ AC2: Using the $wgSpecialContributeSkinsDisabled configuration flag we have added in T319240 should also be applied to the menu item, so that the default experience is to navigate to the contributions entry point page.

Screenshot 2022-11-09 at 7.05.22 PM.png (803×1 px, 272 KB)

Thanks. That solution makes a lot of sense. But:

  • How was I, or anyone else not on your team, supposed to know that the config setting is temporary?
  • What is the purpose of the config setting if you already have a better way of achieving the same thing?
  • Who and when is going to work on that? If the config setting is going to make it into a MediaWiki release, then it needs to work correctly.

This is all good feedback :). I think one of the challenges here is we now have 4 teams operating on this code from different angles. I've opened a ticket: T323083 Feel free to reference it inside MediaWiki core inline comment.

Edtadros subscribed.

Test Result - Prod

Status: ❌ FAIL
Environment: enwiki
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

❌ AC1: Currently, there is a link to Special:Contributions in the user menu. In T319240, we will conditionally show tabs inside the Special:Contribute page.
I could not get the tabs to appear

Screenshot 2022-11-19 at 9.26.19 AM.png (522×1 px, 65 KB)

✅ AC2: Using the $wgSpecialContributeSkinsDisabled configuration flag we have added in T319240 should also be applied to the menu item, so that the default experience is to navigate to the contributions entry point page.
I took a few screenshots to show the current state. I'm not sure that they are valid tests since AC1 failed

Screenshot 2022-11-19 at 9.29.20 AM.png (856×1 px, 221 KB)
Screenshot 2022-11-19 at 9.28.22 AM.png (1×1 px, 285 KB)
Screenshot 2022-11-19 at 9.29.49 AM.png (782×1 px, 236 KB)

We can't test this in production right now, so the fail in production is a good sign, so moving to sign off!