Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator

Add user settings frontend JavaScript API
Closed, ResolvedPublic

Description

It's sometimes useful to store volatile preferences outside stored user preferences. Typically this is done in cookies which [1] we should probably move away from in preference for localStorage

MobileFrontend has a library mw.mobileFrontend.require( 'settings') that saves/gets settings from local storage with optional cookie fallback.
Flow has its own one and there have been various other attempts [2] to do this. Does VE do anything similar?

We should standardise on a lean library that does this in core.

[1] https://gerrit.wikimedia.org/r/#/c/146784/
[2] http://www.cookielaw.org/the-cookie-law/

Event Timeline

Jdlrobson raised the priority of this task from to Needs Triage.
Jdlrobson updated the task description. (Show Details)
Jdlrobson added subscribers: Jdlrobson, TheDJ.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Cookies should only be used for things that the server needs to read when handling an HTTP request. Anything else should be in preferences or one of the HTML5 storage APIs.

While the "cookie law" is somewhat relevant, I wouldn't be too sure about localStorage being any different. The web standard makers and browser vendors certainly do not treat cookies differently than any other type of storage. And I'd agree with them. While localStorage is not sent along the HTTP request, client-side scripts can persist data about the users, even across domains, just the same.

My angle is more towards semantic correctness and performance. Saving data in cookies means they're sent along to the server every single HTTP request. E.g. cookies like "mw_tochide=1", "uls-previous-language=en", "wikieditor-0-advanced=expand" are sent to the server when requesting "https://en.wikipedia/wiki/Page_name". For that reason alone we should avoid cookies.

However expanding use of localStorage in any way is blocked on T66721. Using it now is unreliable and strongly discouraged. Unless the data being stored is purely cache, if it is state, it would lead to noticeable UX problems when the data isn't being saved. E.g. banners show up when they shouldn't, language settings are gone after refresh, sections expanding/collapsing wrongly, etc.

(By the way, what is the use case for settings which are not saved in MediaWiki user preferences, other than supporting anonymous users?)

@matmarex, i can think of two things:

  • session specific settings (so sessionStorage over session cookies)
  • UI state, which makes more sense to bind to your computer, than it makes sense to bind to your user account.
EBernhardson subscribed.

@Krinkle - yup but sadly right now cookies are used for more than that. I think we all agree we should move away from that.

@matmarex in mobile we use it to capture things such as "we've already shown you this." (similar to what fundraising banners do - only they use cookies). Another idea would be to keep an edit in there so you don't lose it when you refresh the page.
Special:Book uses it extensively for collections.

Note I have also just discovered we have jquery.jStorage in core.... sigh.

(Can't see anything about VisualEditor in here; please correct if I erred.)

Just in case there is some overlap between this and previous GSoC work on preferences, see:
https://www.mediawiki.org/wiki/User:Salvatore_Ingala/Notes

So.. I'm hitting this again. I don't want yet another extension depending on MobileFrontend so I beg you can we please define some kind of api in core, who's implementation details use localStorage, falling back to cookies.

I understand T66721 is a problem but it hasn't caused us any known issues in either Gather or MobileFrontend.

Basically we need to keep a local record of whether we've shown something to the user and I really don't want to use cookies and I don't want to reinvent the wheel or add yet another extension that depends on MobileFrontend because of this shortcoming in core...

I understand T66721 is a problem but it hasn't caused us any known issues in either Gather or MobileFrontend.

I still see:

19:58:48.623 "NS_ERROR_DOM_QUOTA_REACHED: Persistent storage maximum size reached" DOMException [NS_ERROR_DOM_QUOTA_REACHED: "Persistent storage maximum size reached"

on every request, so I don't plan on using localStorage for the time being.

@MattFlaschen which browser are you using? In Chrome localStorage is 5MB so that's a lot of raw JS. I highly doubt the average user is hitting this issue, and that this relates to you being a power user. Could you check what's actually in localStorae?

We need some kind of standard API here, to prevent reinventing the wheel in different code repositories (Flow (last time I checked), MobileFrontend, and now QuickSurveys.

The API can throw an exception when localStorage is full or not supported and it is expected the caller deals with this.

Given T101732 is on its way let's not block on this. I will write a patch.

I guess, that Mattflaschen uses Firefox and is very active in multiple language projects of Wikipedia which results in a problem with Firefox's localStorage policy, see https://bugzilla.mozilla.org/show_bug.cgi?id=1064466

@Mattflaschen, @Florian: I also see that localStorage error everywhere. It is tracked at T66721: mw.loader.store should not occupy all of localStorage.

Yup. I recognise this is an issue but I am saying that this issue mostly impacts power users (especially on Firefox). I'm yet to see this on the mobile site, where gadgets and user scripts are currently not available nor used.

Right now MobileFrontend / Gather / QuickSurveys/ Flow are using this mostly for new users for things such as onboarding and surveying, and to avoid re-inventing the wheel I am keen to get an API setup noting this caveat. This could also be used by CentralNotice and make some performance gains by avoiding cookie usage.

When T101732 is fixed that will in turn fix T66721, and if this is complete we will also have a useful API to make use of.

Change 230835 had a related patch set uploaded (by Jdlrobson):
Add simplified user settings API

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

@MattFlaschen which browser are you using? In Chrome localStorage is 5MB so that's a lot of raw JS. I highly doubt the average user is hitting this issue, and that this relates to you being a power user. Could you check what's actually in localStorae?

As has been discussed, it only happens in Firefox. Visiting more than one language Wikipedia is not that unusual (I only do it for testing, but our users do it for real work).

That said, I'm not trying to block this feature, but I'm also not going to use localStorage directly or indirectly until that issue is resolved.

Also, Flow does not use this type of storage as far as I can tell. We use user preferences instead for a very small number of preferences.

Yup. I recognise this is an issue but I am saying that this issue mostly impacts power users (especially on Firefox). I'm yet to see this on the mobile site, where gadgets and user scripts are currently not available nor used.

Eh, I don't think that it mostly impact power users, it can impact even occasional editors too.

As an example:

  • Visit https://pl.wikipedia.org/wiki/Ostatnia_droga_Temeraire%27a, which is a pretty generic article on the Polish Wikipedia, as anonymous user. Run JSON.stringify( mw.loader.store ).length to see how much data ResourceLoader puts in localStorage (this is 707573 for me on first page load).
  • Open the MediaViewer, which is not a power-user feature. The length increases to 923750.
  • Open the VisualEditor, which is not a power-user feature either. The length increases to 2008078.

This is already 2 MB, and this is just one wiki and an anonymous user (I'm not sure how the browsers calculate the limit, but if anything, this is less than the number of bytes actually stored). Visit another wiki (say, the same article in English, if you're not satisfied with the content in Polish and speak two languages) and this doubles. Log in (which gets you features like Echo and lets you enable some gadgets or beta features), or visit a third wiki, and you're quickly over the 5 MB limit.

Change 230835 merged by jenkins-bot:
Add simplified storage API

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