|mediawiki/extensions/DismissableSiteNotice : wmf/1.32.0-wmf.19||Revert "Use session storage instead of cookies for site notices"|
|mediawiki/extensions/DismissableSiteNotice : master||Revert "Use session storage instead of cookies for site notices"|
|mediawiki/extensions/DismissableSiteNotice : master||Use session storage instead of cookies for site notices|
|Resolved||D3r1ck01||T199274 Use localstorage for sitenotices instead of cookies|
As examples, I recently converted geonotice: https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-geonotice-core.js&diff=846272869&oldid=716066008
and watchlistnotice: https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-geonotice-core.js&diff=846272869&oldid=716066008
Note that I use session storage as a backup, for privacy mode and cookie blocking browsers/extensions. Dismissible notice is less used, so maybe it isn't important there. Not sure how often dismissible site notice is used to begin with if im honest.
Patch Set 4: Code-Review-1
Please check the note on T110353, with regards to not having a way to expire the data, which means it builds up indefinitely. We may want to consider mw.storage.session instead if the shorter expiry is acceptable (note that sessionStorage in browsers can actually persist as long as 30 days or longer even, given that browsers tend to resume previous sessions even upon restarts).
Alternatively, we could make an exception given it's only a single key, but it's not great given that'll be what every extension does for their "one" small key/value, also with regards to site admins removing the extension and the data remaining permanently.
- Do we need that when a user dismisses a site notice, it should resurface again after a while? If so, is there a standard duration? e.g. 30 days for example?
- localStorage is almost same as sessionStorage and using the former, it keeps data indefinitely but the later can of-course expire, so should we go for it?
- Why should a user dismiss a site notice and it appears again after a while? Would love to know the reasoning behind this.
Thanks for the review and once I get your feedback, I'll continue work on the patch :)
@D3r1ck01 The reason for expiration is not about seeing the same notice again. It is about not storing data longer than needed. If wiki A creates a notice "1" and enables it, and then disables it after a few weeks, and wiki A does not create another notice for 3 years, then we do not need to store forever that you've seen "notice 1".
Also, if you do not visit a wiki very often, we should allow the browser to delete the data at some point.
You can multiply this with the number of wikis you visit, and multiply with the number of features that exist that can toggle something (table of contents, watchlist notice, site notice, WikiEditor toolbars, "templates used on this page" edit page, etc. etc.). Also keep in mind that gadgets can also create this kind of functionality, and gadgets are sometimes removed, abandoned or disabled. This again causes more data to stay around indefinitely. To prevent the storage from getting full (it is limited to 5 MB in total), we should avoid storing data without an expiry.
The DismissableSiteNotice extension currently uses 30 days for storing the data (in cookies). This is a very common default and works well. However, unfortunately, localStorage does not have a way to expire data. So our only choice is:
- Keep "forever".
- Keep for the session.
Given that DismissableSiteNotice only stores 1 piece of data, and is an extension that tends to be installed always, it is probably safe to use "forever" without wasting data (given the only way for the data to be redundant is if the wiki is not visited often, or if the extension is uninstalled).
On the other hand, perhaps sessionStorage is good enough because it seems sessions can survive quite long, perhaps even longer than 30 days?
Thanks so much for the explanation @Krinkle. While on one hand, cookie expiration time can be explicitly set (in the previous case 30 days) in the source code and we no longer need this solution, on the other hand, sessions can really persist as long as the browser is open, can also survive page reloads and restores. If a wiki page is opened in a new tab or window, a new session will be initiated so this means closing the window or tab will expire the previous session (and is this something we want to do?). See: https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API.
Since we do not need to keep this indefinitely as the extension maybe uninstalled etc (as you've already mentioned above), I think a reasonable solution here will be using sessionStorage. In this light, I'll update the patch and submit a PS :)
Users on nlwiki are complaining they can't hide the sitenotice: https://nl.wikipedia.org/wiki/Wikipedia:De_kroeg#Suriname_en_de_Caraïben
I don't see dismissSiteNotice appearing in my local storage after dismissing the sitenotice.
It does add it to sessionStorage and it doesn't appear again when you normally refresh or navigate. However, if you open a new page (in a tab), it appears again and the entry in sessionStorage is gone. Using Safari 11.1.2 on Mac OS 10.13.6.
Edit: I know read that this is intended behaviour for sessionStorage, quite annoying.
We have the same issue here about opening the page in a new tab: the site notice re-appears. This worked correctly previously: the site notice would normally remain dismissed if the user dismissed it in the previous tab. Please if you could fix it, it would be great for saving the precious screen space and increasing productivity.
EDIT: I can reproduce this issue in Google Chrome 68.0.3440.106 (Official Build) (64-bit) on Windows 7.
EDIT: also reproducible in Firefox
@Gryllida, @Sjoerddebruin, we've reverted the change from "session storage" to "cookie storage" (via a SWAT deploy window) to mitigate the issue you've reported. Thanks! Now we should have cookies running around again :). See this: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/DismissableSiteNotice/+/457516.