Page MenuHomePhabricator

Fix issue with LocalStorage holding outdated dataTypeStore when new data types are added to a wikibase
Closed, ResolvedPublic

Description

When enabling a new datatype on a wikibase, browser clients have a hard time adapting to the change as the new datatype will not automatically get added to the datatypestore held in the browsers local storage.

So for example, when enabling the musical-notation data type on a wiki, if a user has previously used the wiki they will not be able to use the data type unless they clear their local storage.

Instead when they try to edit or add something with the data type the following will happen:

image.png (816×1 px, 94 KB)

Found property P3 in entityStore but couldn't find the datatype "musical-notation" in dataTypeStore. This is a bug or a configuration issue.

The error is thrown from: https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/master/view/resources/jquery/wikibase/snakview/snakview.variations.Value.js#L191-L195

We need to clear and or update localstorage in this error condition, or prompt the user to do so?

Reviewer test steps

  1. Install and enable Score extension
<?php

// in your LocalSettings.php

wfLoadExtension( 'Score' );
$wgMusicalNotationEnableWikibaseDataType = true;
$wgScoreLilyPond = '/usr/bin/lilypond';
  1. create a property of data type Musical Notation (provided by Score extension)
  1. add a property statement of the musical notation type you just created in an item/lexeme

here's a valid lilypond example

\relative c' { \clef treble \key c \major \time 4/4 a4 b c d e f g a2 }
  1. disable musical notation data type
<?php

// in your LocalSettings.php

$wgMusicalNotationEnableWikibaseDataType = false;
  1. hard reload your item/lexeme page with the lilypond property statement

musical notation property won't render now and it'll appear like following screenshot

image.png (160×791 px, 10 KB)

  1. try to edit the musical notation type property

you should see a warning like in the following screenshot

image.png (251×325 px, 11 KB)

  1. enable musical notation data type again
<?php

// in your LocalSettings.php

$wgMusicalNotationEnableWikibaseDataType = true;
  1. hard reload your item/lexeme page with the lilypond property statement
  1. try to edit the musical notation type property again

you should be able to edit the property now, and should not see a warning similar to the one form step 5

Event Timeline

High as this blocks the deployment of the musical notation datatype (at least without breaking stuff for people)

@Addshore

I've been investigating this one for a while. It seems that data types are not really stored in localStorage.
I checked locally, on beta and on production but couldn't find any localStorage entries apart from very basic ones that do not store data types.

Since I have the data type enabled on my local instance, and haven't experienced the same issue before
(even with caching settings all kept turned on), I tried to reproduce it and could only do so by following these steps:

  • create a musical notation statement for some lexeme.
  • disable the extension entirely
  • purge&load the lexeme page
  • then you'll see error formatting the score property (missing datatype)
  • try to edit the score statement
  • then you can see similar error in js console

and given that situation, only enabling the extension and following these steps would fix it again:

  • load the lexeme page (do not purge)
  • then you'll see error formatting the score property (missing datatype)
  • try to edit the score
  • then you don't see and error in js console
  • purge&load the lexeme page
  • then you'll see the score rendered correctly
  • try to edit the score property
  • then you don't see an error in js console

The resource loader already have ETag header being generated, and DataTypesModule modifies the final
version when data types list is changed (only the keys).

This is leaving me thinking that either:

  • I'm missing something regarding when/how resource loader might be caching responses on the frontend, or
  • the issue is actually caused by cached pages on the BE rather than localStorage entries on the FE.

We can have look together this week when you are back, or before then if you get the chance to add more info on how you can reproduce it that'd be great!

Stalled because cannot reproduce the claimed case. Need more info.

Turns out that for Firefox we do not store resource loader response in local storage https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/master/resources/src/startup/mediawiki.js#2216

So on chrome the issue could be reproduced by following the steps:

  1. disable the extension after creating at least one property in an item with musical-notation type
  2. clear browsing storage (localStorage at least)
  3. reload lexeme page
  4. try to edit the score property
  5. then you should see a similar js error Found property P2 in entityStore but couldn't find the datatype "musical-notation" in dataTypeStore. This is a bug or a configuration issue.
  6. enable extension again
  7. reload (do not hard reload) the lexeme page and try to edit the score property
  8. then you should see again the same js error
  9. remove entry in local storage with key 'MediaWikiModuleStore:default'
  10. reload again
  11. try to edit score property
  12. the js error should not occur and you should be able to edit the property

Note To double test too. When I tried to reload at step 10, backend kept producing old ETag for few more hits causing cached version on disk to be loaded and reproducing the issue again

So the final thing I found out is that we actually have a 5 minutes max age for resources loaded via the resource loader. Even clearing local storage alone won't fix this issue unless you either clear all browsing cache or force bypassing reloading a page (keys ctrl+F5 on most browsers).

IGNORE THIS COMMENT .. 5 minutes isn't always true

@Addshore can you please do a sanity check and follow your steps to reproduce the issue, but this time just wait longer that 5 minutes between re-enabling the extension and then refreshing again? (you do not even need to by pass your caches nor clear localstorage, it is already taken care of in our MW ResourceLoader and Store dance).

For a visual clue, check the dev tools like in this screenshot

image.png (490×2 px, 158 KB)

when Size column shows the size again, then it loaded it from server again as 5 minutes have passed.
You can also check for the 'musical-notation' datatype being loaded into FE by executing mediaWiki.config.get('wbDataTypes')['musical-notation'] in cosole

@Lydia_Pintscher ? I would actually change nothing here then.. as 5 minutes caching policy isn't a big deal for new datatypes. Probably no one will add/edit properties of a newly added data type just on the first 5 minutes.. am I being too cheap here?

@Lydia_Pintscher For now the quick fix on our side is to:

  1. clear resource loader local storage on client side
  2. prompt for the user to inform them they need to refresh the page to get an updated version of client.

Sounds good? If yes, what copy would you like to use to say "Oops, looks like your client version is outdated. Please refresh the page to get an updated version and edit properties of type '{musical-notation}'"


For a long-term solution, I will create a separate ticket on mediawiki project to basically allow reloading resources when they are outdated, but I still need to figure out what a proper solution might be.

Change 493702 had a related patch set uploaded (by Alaa Sarhan; owner: Alaa Sarhan):
[mediawiki/extensions/Wikibase@master] Warn to hard refresh page when datatypes in client side seems outdated

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

Change 493702 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Warn to hard refresh page when datatypes in client side seems outdated

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

@Lydia_Pintscher I tried to test this (hackingly) when the change got on beta:

modified my localStorage by hand to delete the new datatype, refreshed softly and tried to edit -> got the warning -> hard refreshed -> new data type was loaded again and I could edit the property.

I think we will have to wait and see if this gets reported back to us, or you wanna ask some beta users to keep an eye on it maybe.

Cool. Let's close this then and if I hear anything from someone who tests we'll look into it again.