Page MenuHomePhabricator

UI breaks on UnDeserializable value.
Closed, ResolvedPublic1 Story Points

Description

If deserializing of a value fails and it is replaced by an UnDeserializableValue. PropertyValueSnakFormatter renders this as <span class="error wb-format-error">...</span>.

UI initialization fails to generate an [edit] link for the statement, and appears to just crash/abort. No [edit] links are shown after the undeserializable statement on in the EntityView.

Event Timeline

daniel created this task.Mar 17 2015, 4:11 PM
daniel raised the priority of this task from to Normal.
daniel updated the task description. (Show Details)
daniel added a subscriber: daniel.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 17 2015, 4:11 PM

@daniel: Which project is this about? Associating one is very welcome (e.g. MediaWiki-General ).

Krenair added a subscriber: Krenair.

Guessing Wikidata. @daniel, please associate projects when creating tasks, otherwise the relevant people will not see the task.

@lydia The trigger seems quite different, but the underlying issue appears to be the same: the JS UI is very brittle. It should implement safe points for recovery at least after snacks, but perhaps also after statements, and statement groups, and sections.

Tobi_WMDE_SW renamed this task from UI breaks on UnDeserializable value. to Investigation: UI breaks on UnDeserializable value..Apr 21 2015, 1:40 PM
aude claimed this task.Apr 21 2015, 8:54 PM
aude moved this task from Backlog to Doing on the Wikidata-Sprint-2015-04-21 board.
aude added a comment.Apr 22 2015, 6:46 PM

There are various inconsistencies in how we handle "undeserializable" values in the backend and in the frontend.

One case that I found are time values with precision higher than a day (e.g. minutes or seconds). The validator would not accept such new values, but the backend is able to handle these when rendering a page and the value is not displayed as invalid.

The front end deserializes the entire wbEntity json blob into javascript objects and data values. The javascript code does not handle time values higher than a day. If there is an error in deserializing a snak, then an "UnUnserializableValue" is created and then causes js to break when creating snakviews.

The problem with the "UnUnserializableValue" is that it does not yet implement methods "toJSON" and "newFromJSON". These should be implemented, and then the UI does not break on page view.

More work needs to be done to handle editing these in a decent way.

Yet further work is needed to make handling of "UnDeserializableValues" (those generated from the backend) and those in the JS more consistent, and nicer.

related issue is the name "UnUnserializableValue" in the JS. This is actually fixed on DataValuesJavascript master, and looks like https://github.com/wmde/DataValuesJavascript/pull/73 might help with the "invalid" time values.

We need to finish up DataValuesJavaScript 0.7.0 and release it, and adapt ValueView (already is a pull request) and other components for the changes https://phabricator.wikimedia.org/T94905

all in all, I don't see a quick, easy, nice fix for the issue. In my opinion, we need to work on the above things.

I assume this can be closed as the investigation has been done. @aude @Lydia_Pintscher We should consider the tasks mentioned by @aude for the next sprint..

aude added a comment.May 7 2015, 5:55 PM

unless we have another task for the actual bug, then I would keep this open and remove "investigation" from the description. otherwise, we may forget about this bug.

aude renamed this task from Investigation: UI breaks on UnDeserializable value. to UI breaks on UnDeserializable value..May 7 2015, 5:55 PM
aude added a comment.May 21 2015, 3:39 PM

I have resorted to working on a branch of Wikibase for this, that brings all the pieces together:

http://git.wikimedia.org/tree/mediawiki%2Fextensions%2FWikibase.git/refs%2Fheads%2Fdv070

once we get components tagged and all, I can make an actual patch for this.

Tobi_WMDE_SW edited a custom field.May 25 2015, 11:00 AM
daniel raised the priority of this task from Normal to High.Jun 1 2015, 12:56 PM

*bump*

Change 212515 had a related patch set uploaded (by Aude):
Use UnDeserializableValue expert for handling such values

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

Change 212515 merged by jenkins-bot:
Use UnDeserializableValue expert for handling such values

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

Change 219363 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Fix docs in snakview.js

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

Change 219363 merged by jenkins-bot:
Fix docs in snakview.js

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

Bene closed this task as Resolved.Jun 22 2015, 7:29 PM
Bene added a subscriber: Bene.