Run validation on charts before render
Closed, ResolvedPublic3 Estimated Story Points
Actions

Assigned To

Authored By

	CCiufo-WMF
	Mar 24 2025, 6:44 PM

Description

Acceptance Criteria

Page renders with error box displays when invalid charts are encountered (T389125, T389126)
Add appropriate error tracking category if applicable
Integration test that confirms page can render with error

Sign off

Review tracking category behaviour.
Ensure fix solves T389126
Ensure fix solves T389125

Details

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
Add tests for charts with invalid tabular data	mediawiki/extensions/Chart	master	+645 -88
Handle invalid json data pages in chart parser function	mediawiki/extensions/Chart	master	+728 -84
Handle invalid json data pages in chart parser function [bugfix only]	mediawiki/extensions/Chart	master	+108 -4

Customize query in gerrit

Related Objects
Search...

Status	Subtype	Assigned	Task
Resolved		aude	T389857 Run validation on charts before render
Resolved	BUG REPORT	CCiufo-WMF	T389126 Possible to break a page with invalid data
Resolved	BUG REPORT	CCiufo-WMF	T389125 Possible to break a page by passing in a chart that is not a JSON

Event Timeline

CCiufo-WMF created this task.Mar 24 2025, 6:44 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 24 2025, 6:44 PM

CCiufo-WMF triaged this task as High priority.Mar 24 2025, 6:44 PM

CCiufo-WMF added a parent task: T389125: Possible to break a page by passing in a chart that is not a JSON.

CCiufo-WMF added a parent task: T389126: Possible to break a page with invalid data.

CCiufo-WMF set the point value for this task to 3.

CCiufo-WMF assigned this task to aude.Mar 24 2025, 6:47 PM

Jdlrobson-WMF renamed this task from [Placeholder] Run validation on charts before render to Run validation on charts before render.Mar 24 2025, 6:57 PM

Jdlrobson-WMF removed a parent task: T389126: Possible to break a page with invalid data.

Jdlrobson-WMF added a subtask: T389126: Possible to break a page with invalid data.

Jdlrobson-WMF removed a parent task: T389125: Possible to break a page by passing in a chart that is not a JSON.

Jdlrobson-WMF added a subtask: T389125: Possible to break a page by passing in a chart that is not a JSON.

Jdlrobson-WMF updated the task description. (Show Details)

Change #1130720 had a related patch set uploaded (by Aude; author: Aude):

[mediawiki/extensions/Chart@master] WIP - Handle invalid json data pages in chart parser function

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

gerritbot added a project: Patch-For-Review.Mar 24 2025, 8:56 PM

JCDataContent::getLocalizedData does a validation check and may return null if the data is invalid.

We are missing a check for this (null value) for both tabular data content and chart definition content.

https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/JsonConfig/+/refs/heads/master/includes/JCDataContent.php#94

Also, there is an overall lack of tests in the Chart extension. I can add tests for these specific cases but it would be good to add more test coverage in general in the extension.

aude moved this task from Incoming to Doing on the Charts (Current Sprint) board.Mar 24 2025, 10:15 PM

Change #1132000 had a related patch set uploaded (by Aude; author: Aude):

[mediawiki/extensions/Chart@master] Handle invalid json data pages in chart parser function

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

Change #1132000 merged by jenkins-bot:

[mediawiki/extensions/Chart@master] Handle invalid json data pages in chart parser function [bugfix only]

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

aude mentioned this in rECRTb9e062d4860d: Handle invalid json data pages in chart parser function [bugfix only].Mar 28 2025, 6:21 PM

ReleaseTaggerBot added a project: MW-1.44-notes (1.44.0-wmf.23; 2025-04-01).Mar 28 2025, 7:00 PM

Change #1133573 had a related patch set uploaded (by Aude; author: Aude):

[mediawiki/extensions/Chart@master] Add tests for charts with invalid tabular data

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

Change #1130720 abandoned by Aude:

[mediawiki/extensions/Chart@master] Handle invalid json data pages in chart parser function

Reason:

Replaced by 1133573 for the tests

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

Let's break out the testing into a separate task (maybe as part of T390103) so that we can move forward with signing off on the fix.

This seems to have fixed T389126, although I'm not sure why the page renders with "⧼chart-error-invalid-source-content⧽".

From https://commons.wikimedia.beta.wmflabs.org/w/index.php?title=Data:Break.chart:

It doesn't seem to fix T389125 though, I'm still getting a MediaWiki internal error at https://commons.wikimedia.beta.wmflabs.org/wiki/Data:1993_Canadian_federal_election_localized.chart.

bvibber subscribed.Apr 29 2025, 6:14 PM

Seddon updated the task description. (Show Details)Apr 29 2025, 6:40 PM

CCiufo-WMF closed subtask T389126: Possible to break a page with invalid data as Resolved.May 2 2025, 3:07 PM

CCiufo-WMF closed subtask T389125: Possible to break a page by passing in a chart that is not a JSON as Resolved.

CCiufo-WMF updated the task description. (Show Details)

CCiufo-WMF closed this task as Resolved.May 5 2025, 5:38 PM

CCiufo-WMF moved this task from Doing to Ready for Signoff on the Charts (Current Sprint) board.

Change #1133573 abandoned by Aude: