Change 957378 had a related patch set uploaded (by Jdrewniak; author: Jdrewniak):

[mediawiki/core@master] Enable clientPreferences for logged-in users

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

Change 957379 had a related patch set uploaded (by Jdrewniak; author: Jdrewniak):

[mediawiki/skins/Vector@master] Save limited-width toggle to client-preferences for logged-in users

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

[…] preferences should work for logged in users as the current behaviour is confusing and the capability is equally useful for logged in users as it is for anonymous users.

This is a non sequitur. Whether Vector saves the preference for logged-in users (like it does for client width), and whether the skin feature exists are two unrelated decisons. What's preventing Vector from saving the font-size preference like it does for client width?

Web team chatted about this and decided that client preferences should work for logged in users […]

This isn't a decision for your team to make.

Client preferences are very specifically designed and implemented as compromise solution for logged-out users where persisting preferences and server-wide variation is currently not yet supported. Neither of these applies to logged-in page views.

In T345664#9165465, @Krinkle wrote:

[…] preferences should work for logged in users as the current behaviour is confusing and the capability is equally useful for logged in users as it is for anonymous users.

This is a non sequitur. Whether Vector saves the preference for logged-in users (like it does for client width), and whether the skin feature exists are two unrelated decisons. What's preventing Vector from saving the font-size preference like it does for client width?

Web team chatted about this and decided that client preferences should work for logged in users […]

This isn't a decision for your team to make.

Client preferences are very specifically designed and implemented as compromise solution for logged-out users where persisting preferences and server-wide variation is currently not yet supported. Neither of these applies to logged-in page views.

My Personal take on the task: (This is not our team's full insight)
The initial premise here is that some preferences might be by design device specific regardless of user status ie:font-size, and that would:

• Need to Utilise a device specific preference provider.
• Help reduce the amount of data and traffic required to store for these extra preferences.
• Help in supporting the experiment with some preferences, without requiring to add huge amount of data to the preferences table, and reduce the cleanup processes when needed to be removed.

My suggestion:

• We can move all the logic regarding of the inclusion of client preferences from core initially to a function in Skin.php, with the default code as it showing with requiring user not to be registered aka logged out only.
• Then for experimenting and override purposes cases can be covered in function overrides inside the skins.
• Skin handling of preferences should be preference by preference cased as not all need to be device specific for logged in user's.

Hi @Krinkle -
Thank you for sharing your feedback. I understand and appreciate the concerns you’ve raised about the preference persistence mechanism.

To start with, can you say more about your specific concerns with the change? They will be helpful as we begin to consider next steps or alternate solutions.

Stepping back a bit, I agree that we need to be cautious and considerate here, and would say in this particular instance this ticket came about initially in response to a smaller-scoped conversation, but as we’ve considered it more fully it has highlighted the need to discuss this more holistically. In light of this, the Web team has discussed this internally and we want to make sure we clarify certain aspects of this which your reply touches on. In particular:

1. Scope - We want to acknowledge that throughout the process of the TDMP, we had discussed logged-out users only, and recognize that expanding its application to logged-in users is a change in scope. That said, based on our annual plan, we would need to offer preferences for logged-in and logged-out users, so we do need some solution here. While a device-based preference is preferable, we're also open to considering solutions that are based on skin and will want to consider mechanisms such as the user preference table as well. In any case, it's crucial to evaluate the implications of this thoroughly, particularly in light of IP Masking, any necessary migration between user states, and DB concerns around the user_preferences table. Next steps:
   • We will write up a proposal of how to proceed with building out preferences for logged-in users and share it with the MW team for further refinement.
   • We will scope out other ways to store the preference. We have already created https://phabricator.wikimedia.org/T346375 in which @Ladsgroup gave us valuable feedback about potential considerations for storing this kind of thing in user_preferences – based on this reply, it sounds like we will need to think about other options in this space.
2. Requirements - We need to be clear as to the product requirements for the font-size feature, specifically as it relates to device-specificity, user-specificity, and skin-specificity. Next steps:
   • We will further document the requirements here, and in light of these, note pros and cons for the options on the table and our reasoning. We will look to coordinate with all involved parties to over time define general principles we can refer to in future decision-making in this space.
3. Ownership - There has been some discussion that the MW team will eventually take ownership of this feature. The timeline and transition process, however, is something that needs additional clarity. Collaborative alignment on this is essential to ensure a smooth handover and to avoid any operational pitfalls. Next steps:
   • Olga and Birgit will convene and discuss a timeline and roadmap for transition of this feature and plans for the interim.

I overall wanted to highlight that since preference persistence is a new feature, there are still open-ended questions and parameters we have yet to figure out. We're also hoping to begin defining what ownership and collaboration with the new MW team looks like, both now and in the future.

The amount of unknowns makes this process tricky, and extra understanding and patience on all sides while we negotiate these factors is appreciated. We will look to use the above mechanisms to provide additional clarity as to the feature requirements and timelines, and look forward to working together with the Mediawiki team to define what this looks like now and in the future. We will follow up as this evolves.

In T345664#9170904, @NHillard-WMF wrote:

• We will scope out other ways to store the preference. We have already created https://phabricator.wikimedia.org/T346375 in which @Ladsgroup gave us valuable feedback about potential considerations for storing this kind of thing in user_preferences – based on this reply, it sounds like we will need to think about other options in this space.

One thing is that storing in user_properties table for logged-in user preferences has been the canonical place to store such data in the past ten years at least. I said it's blocked because that table is exploding doesn't translate to we need to find a way around it. Adding a new competing system next to the existing one (user_properties) will lead to overhead of maintenance, migration and clean up in the next couple of months or so.

If this is blocking the team, IMHO, we should pool resources to get this fixed ASAP as if it's not fixed soon, it'll cause issues regardless of adding new user prefs to that table or not.

In T345664#9173550, @Ladsgroup wrote:

In T345664#9170904, @NHillard-WMF wrote:

• We will scope out other ways to store the preference. We have already created https://phabricator.wikimedia.org/T346375 in which @Ladsgroup gave us valuable feedback about potential considerations for storing this kind of thing in user_preferences – based on this reply, it sounds like we will need to think about other options in this space.

One thing is that storing in user_properties table for logged-in user preferences has been the canonical place to store such data in the past ten years at least. I said it's blocked because that table is exploding doesn't translate to we need to find a way around it. Adding a new competing system next to the existing one (user_properties) will lead to overhead of maintenance, migration and clean up in the next couple of months or so.

If this is blocking the team, IMHO, we should pool resources to get this fixed ASAP as if it's not fixed soon, it'll cause issues regardless of adding new user prefs to that table or not.

Hi @Ladsgroup - my apologies for summarizing your comments incorrectly. I completely agree about your concerns, and I did not mean to suggest we should develop an alternative mechanism entirely! We will be reaching out to talk further about that other issue and what can be done in this space / how we can work past this. Thank you again for your input and support.

In T345664#9168882, @Mabualruz wrote:

In T345664#9165465, @Krinkle wrote:

[…] preferences should work for logged in users as the current behaviour is confusing and the capability is equally useful for logged in users as it is for anonymous users.

This is a non sequitur. Whether Vector saves the preference for logged-in users (like it does for client width), and whether the skin feature exists are two unrelated decisons. What's preventing Vector from saving the font-size preference like it does for client width? […]

Client preferences are very specifically designed and implemented as compromise solution for logged-out users where persisting preferences and server-wide variation is currently not yet supported. Neither of these applies to logged-in page views.

The initial premise here is that some preferences might be by design device specific regardless of user status ie: font-size

That's fine. If the product requires a third way of expressing preferences (anon pref, logged-in per device, logged-in per account), then that's what you need to do. No problem.

My suggestion: […]

You don't need to develop or deploy any new mechanisms for this. Toggling features by device cookie is an existing capability on the platform that Vector can use.

The server-side can vary its Skin response by cookie in the same way that it does for user preferences and other cookies already. The logic for this is already present in the Vector skin. This kind of variance is problematic for CDN-cached responses but there is no such restriction on responses with an active server session (whether edit session, temp user session, or logged-in session).

For logged-in users there is no need to client-side render. This is why it's critical that the client-pref mechanism remain limited to cached contexts where it is technically neccecary, and thus avoid relying on it more generally as that's is counter to our architecture and direction.

• Help reduce the amount of data and traffic required to store for these extra preferences.

I would not recommend weighing the cost of API requests when making product decisions about user experience. These costs are negligible and drown out by other background activity on the platform, for example: WikimediaEvents instrumentation for DesktopWebUIActionsTracking.

Having said that, the capability to use device cookies is part of the platform and available for use.

In terms of comparison, if the client pref feature were embedded on logged-in page views, it would add the equivalent size of 10-20 feature toggle cookies. It's not much, but, since we're measuring :)

• Help in supporting the experiment with some preferences, without requiring to add huge amount of data to the preferences table, and reduce the cleanup processes when needed to be removed.

I see no evidence that this feature would add a "huge amount of data" to the preferences table. If it was, it would be a bug with a platform capability and mandate improvement.

At this time, both device cookies and user preferences are available to base your feature on and work as intended in ways that that have no observable end-user differences to what you propose. As such, I'm declining the ResourceLoader feature request to deploy client prefs to logged-in users.

Change 959045 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] Disable client preferences by default

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

Change 959013 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@wmf/1.41.0-wmf.27] Disable client preferences by default

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

Sorry for the noise. The 2 patches above were mistagged and unrelated to this ticket.

Change 959358 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] It should be possible to persist Vector font size for registered users

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

After to chatting to @NHillard-WMF I've separated the Vector problem here (making font-size configurable in Vector for logged in users - T346987) from the general concern (the mw.user.clientPreferences API is incomplete) - this task for further discussion.

@Mooeypoo I've had a go at turning this into a problem statement with the 5 possible solutions that I am aware of [1]. What's the best way for us to gather the right people in a room to seek other possible solutions and make a decision here while we do not have an official process (e.g. TDMP)?

[1] @Krinkle please feel free to amend solution 5 if I've not captured your proposal correctly and add pros/cons to other options that you are aware of.

Change 957378 abandoned by Jdlrobson:

[mediawiki/core@master] Enable clientPreferences for logged-in users

Reason:

Abandoning for now until we've reached a decision on the future of this feature.

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

Change 957379 abandoned by Jdrewniak:

[mediawiki/skins/Vector@master] Save limited-width toggle to client-preferences for logged-in users

Reason:

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

Option 1: Do nothing
[…] Cons: The API is counter-intuitive to use as it works differently for logged in and anonymous users.

Option 3 mw.user.clientPrefs should save to a user preference:
[…] Pros: The API works intuitively.

It is common throughout the MediaWiki platform for a primitive to be specific to (or vary by) logged-in status, user rights, user group, or by individual page or namespace. I disagree with the notion that an API is counter-intuitive merely for having a defined scope.

It is explicitly not a design goal for APIs to always work on everything. I find such desire to often lead led to APIs that are, perhaps surprisingly, counter-intuitive. In addition, they are often novel, brittle, subject to high maintenance, regularly "need" breaking changes (justified by making another incompatible thing fit in), and the cascading effect that change has on maintenance and new bugs on consumers of that API. Overcomplicating features beyond their designed and optimal use case, tends to lead to scalability and performance problems due to mixing different use cases. This tends hampers on-boarding and the ability for developer to transfer or re-use their learned skills. An API that does everything for use cases that appear related (but aren't) leads to each overlapping APIs with their own complete yet non-interoperable worldviews.

Scoping an API, in my experience, makes for a reliable and maintainable platform in the long-term. Use of stable primitives tends to lead to learning ground truths, that developers can then intuit, expect, and apply through their own code as well. Being "protected" from such truths is useful if a low-level and high-level distinction can be made, where a sizeable class of developer never needs to be aware of one. "Revision slots" and "ExternalStore" are examples of this in the revision backend. If your extension doesn't interact with MCR (like Wikibase) or with custom storage (like Flow), then you rely entirely on higher-level APIs and never even know these concepts exist.

A leaky abstraction that acts merely as utility function, are great for last-mile code such as an individual extension or skin, but generally makes for poor platform APIs. It gives you a stable contract where you know you only pay for what you need, and where you are insulated from needless churn or changes due to unrelated customers bolting stuff into an ever-growing ever-slower ever-breaking utility function.

The declarative nature of user preferences and HTMLForm, for example, I think has lent itself well to rapid prototyping of entire products in a way that is production-ready from the start. It gets you to market to start learning what you actually need/want instead of spending time re-inventing high-level concepts during early design stages, which delays important product learnings (e.g. due to over-simplifying or incorrectly abstracting a core concept). The iOS history re-design from a few years is an example of that, the design was to provide stats about page history edits with a percentage of edits by bot accounts vs human accounts, i.e. counting distinct accounts, not edits. This design wrongly assumed that "bot" is primarily an account-level status rather than the per-edit concept that it actually is (the "bot" right is granted to many humans as well, and humans and scripted processes alike can choose on a per-edit basis whether to apply their bot flag). This wasn't realised until many months into development after building a non-functional API from scratch, when user testing showed the mental model didn't match reality. Building stable primitives sets you up for success, both in terms of stability/performance, as well as matching ecosystem and user expectations.

Back on-topic, the second half of the description for Option 3 seems to agree with this sentiment, albeit without for different reasons:

Option 3: mw.user.clientPrefs should save to a user preference:
Cons: […]

• The solution would require us having a way to associate a user preference key with every client preference.
• The solution would require us having a way to register client preferences on the server side so that all client preferences have a preference key associated.

Option 2: mw.user.clientPrefs should throw:
[…] Cons: The solution limits our ability to ship preferences for logged-in users […]. Would require [an] API for changing the HTML tag classes for logged in users.

Option 5: A server side method should be provided

I'm confused as to what is "limiting" you or where this seemingly hard "require" is coming from.

Status quo:

• mw.cookie requires no registration, it is called immediately with any key.
• UserOptions has a declarative interface via extension.json that automates everything for you, including a SpecialPage UI, REST API, performant preloaded retrieval client-side via mw.user.options, etc.

MediaWiki has supported skin-level preferences for logged-in users for over a decade, as well as several client-side cookies (e.g. MediaWiki core: table of contents state, EditPage templatesUsed. WikiEditor: open sections. VisualEditor: Remember last editor. Vector: Collapsible sidebar sections.)

Vector itself demonstrates this already as well. I'm sure there's room for improvement, but it certainly seems that mw.cookie and UserOptions are extension interface that people generally find easy and effortless to adopt in their products today.

Having two ways to define user preferences in an extension seems ultimately more confusing and less intuitive, not the other way around. I'm unsure how the two would interoperate. For example:

• W would these auto-magically registered preferences also be available through User->getOption and mw.user.options? And would we support that or would that be considered internal/unstable?
• Would the internal option name format and option value format become public APIs in-themselves that other code may start to rely on? Can we ever change it? What would the back-compatible migration state look like?
• Would other code be allowed to change these values via mw.Api? We have an establish concept of key ownership in mw.cookie and mw.storage. This is not the case with preferences where we encourage general purpose interfaces and APIs to read/reset/change any/all preferences. It sounds like this might require some custom value validation format.
• Would it be supported to make them visible on Special:Preferences? Would it be supported to connect it to an existing user preference that you already have in your code?
• Would the mw.user.clientPrefs become always async become 1 possible use case is async? Or would the method sometimes be sync and sometimes async?

If we centralise this in core, do you envision such abstraction to do something besides hiding an interaction with two systems that you as caller already need to be aware of and interact with?

I worry that this would raise instead of lower the barrier to entry, as would not hide or abstract a dependency. Adding 1 line of code to call two functions with an OR condition seems simple enough, and has the benefit of giving you re-usable transferrable skills and a sense of familiarity. Especially given that mw.user.options is likely used elsewhere in the same codebase already, so its not an unfamiliar method. Imagine the feeling of realising after two years of calling X that it actually does mw.user.options underneath. Would that have been helpful?

If it saves a few lines of code to wrap this in a local utility function, then by all means do so of course, use your own judgement :)

Option 2: mw.user.clientPrefs should throw:

The existing mw.user.clientPrefs.set API would be modified to throw an error when not supported.

Pros: The API is marked as explicitly being unavailable to users. The error helps guide developers to use a user preferences

This sounds good to me.

I do note that the developer docs for mw.user.clientPrefs API explicitly state "only supported for unregistered users". When I documented the system last month, I added that as well as a general explanation of how the system works, with examples of how to use it, and how to integrate it for registered users (change 947480). This matches how Vector uses it already in production.

Change 970270 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] mediawiki.user: Throw from clientPrefs.set() for registered users

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

Change 970270 merged by jenkins-bot:

[mediawiki/core@master] mediawiki.user: Throw from clientPrefs.set() for registered users

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

Hi Timo -
Thanks for weighing in on the approach here and sharing your thoughts.

We noticed the patch was merged and were wondering whether that was intentional. Overall, the approach seems similar to Option 2 outlined above, which we think is potentially a good way forward, but with some caveats that we wanted to speak further about.

In particular, the change seems to have caused a regression in functionality we are currently working on with respect to user-initiated font-size and limited width customizations:

To replicate:
Go to https://en.wikipedia.beta.wmflabs.org/wiki/Spain
Sign in
Click the settings cog in the bottom right corner
Click any of the options under “type size”, such as “Small”

Expected:
Utilizing this API, the font size of the typography on the page would be altered

Actual:
The controls for font size and limited width no longer work

This is a key part of the functionality we are building out now for the Reading Accessibility project (Key Result WE 2.1 of the Annual Plan) so it’s crucial that this use case is supported in the API.

With that in mind, could we revert this patch for now, and have some time to propose some adjustments here and in Vector 2022 before merging it again? We would also be open to a working session between our two teams to work out a solution together. We can use our upcoming meeting this Thursday to better define what this could look like.

In the meantime we plan to provide our product requirements and open questions in a separate document, which will hopefully clear up any confusion around our needs from the API. We recognize this information has so far been spread across a number of phabricator tickets, which might have been a bit difficult to understand. Please feel free to ask any questions you may have as well.

@Jdlrobson I can't help you if you use unsupported features. This patch was carefully merged on a Tuesday after the branch cut. That gives you the full 6 days to write a 1-line patch that fixes the bug in Vector by adding a one-line conditional statement to comply with what was already the previously agreed and documented contract. I believe this is reasonable and the more efficient approach than to revert, fix, revert-revert.

Upon closer look, I note that this issue appears to be specific to a non-default beta feature. The steps to reproduce don't mention it, but to reproduce the issue you describe require one to first enable the Vector 22 beta feature. This beta feature doesn't exist in production today from what I can see, not even as an opt-in. Would this affect production as-is even if it wasn't addressed in six days?

Change 970803 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] POC: Separate HTML class manipulation from storage

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

Change 972917 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/Vector@master] POC: Fix client preferences for logged in users

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

FYI: We spun out a subtask for the product fix here: T351447

Change 972917 abandoned by Jdlrobson:

[mediawiki/skins/Vector@master] POC: Fix client preferences for logged in users

Reason:

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

Change 970803 abandoned by Jdlrobson:

[mediawiki/core@master] POC: Separate HTML class manipulation from storage

Reason:

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

Untagging this as web team got answers around how to proceed for now. I will keep Moriel up to date with developments and we'll report back if we see duplicate code arising.

Summary from the MW Engineering Group after some discussion about this: As far as the MediaWiki Engineering side of things, this is done.

ResourceLoader itself cannot be the tool that unifies operation between logged-in and logged-out users in this case:

• There are functional differences between logged-in/logged-out users in the features. Upstreaming the abstraction to ResourceLoader won't save features from checking (and providing separate behavior) for either audience.
• Expectations from ResourceLoader are not consistent: Other features that save information or preferences for logged-out users (like VisualEditor) have different expectations and behavioral needs. If we upstream the abstraction to ResourceLoader, it will need to be generalized behavior, and our examination of different needs showed that won't be straightforward, if possible.
• Keeping the behavior in feature is relatively simple: Given the potential complexity of upstreaming the behavior to the platform, the need doesn't seem to justify the added work and maintenance that would follow for ResourceLoader. The feature method is a couple of lines, and upstream won't reduce it by much.

As shepherds of the upstream platform, our recommendation is to keep the behavior as-is, handle the differences in the feature side, and provide abstractions in the product itself that can be used by other features. Similar to other libraries, if the abstraction is used by multiple use cases with multiple products in the future, we can discuss the opportunity (or viability) of potentially upstreaming the behavior to ResourceLoader, similar to how we handle these in other libraries.

Thank you for the summary!