aminghadersohi commented on code in PR #45575:
URL: https://github.com/apache/superset/pull/45575#discussion_r4367130997
##########
superset-frontend/plugins/plugin-chart-echarts/test/Rose/transformProps.test.ts:
##########
@@ -193,3 +196,27 @@ test('renders when __timestamp is serialized as a
non-epoch string', () => {
expect(seriesNames).toEqual(['East', 'West']);
expect(series[0].data.map((d: any) => d.value)).toEqual([30, 10]);
});
+
+test.each([true, false])(
+ 'escapes literal series text in Rose tooltips (rich=%s)',
+ richTooltip => {
+ const name = 'Love <3 charts & <img src=x>';
+ const props = new ChartProps({
+ width: 800,
+ height: 600,
+ formData: { ...formData, richTooltip },
Review Comment:
Fixed in 214f0b4ce2781a32c33909c62154e017253e8670.
I set `dateTimeFormat` to `'<b>period</b>'` in the regression test 'escapes
literal series text in Rose tooltips (rich=%s)' and assert the complete literal
text with exactly one structural `<b>`. I reproduced the gap: with the original
test, removing either period-label escape independently still passed. With the
strengthened test, each removal fails its corresponding rich/plain case;
restoring the escape passes.
The sibling sweep also supplies markup from registered number formatters and
translated range text. All 18 individual escape-removal mutations fail DOM
assertions, including the drilled Rose formatter in 'drilled Rose tooltips
preserve literal series and period text'. Restored-source Jest: 3 suites, 32
tests passed (`CI=true npm run test -- --runInBand
plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts
plugins/plugin-chart-echarts/test/Rose/transformProps.test.ts
plugins/plugin-chart-echarts/test/Rose/EchartsRose.test.tsx --coverage
--coverageReporters=json`).
Siblings checked:
-
superset-frontend/plugins/plugin-chart-echarts/test/Rose/transformProps.test.ts:207
— fixed in 214f0b4ce2781a32c33909c62154e017253e8670
-
superset-frontend/plugins/plugin-chart-echarts/test/Rose/EchartsRose.test.tsx:190
— fixed in 214f0b4ce2781a32c33909c62154e017253e8670
-
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:442
— fixed in 214f0b4ce2781a32c33909c62154e017253e8670
-
superset-frontend/plugins/plugin-chart-echarts/src/Rose/transformProps.ts:136 —
not affected: series-name and formatted-value escapes are present; removing
each separately fails the Rose tooltip regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Rose/transformProps.ts:141 —
not affected: rich period-label escape is present; removing it fails only the
rich case.
-
superset-frontend/plugins/plugin-chart-echarts/src/Rose/transformProps.ts:183 —
not affected: plain period-label, series-name and formatted-value escapes are
present; removing each separately fails the plain case.
- superset-frontend/plugins/plugin-chart-echarts/src/Rose/EchartsRose.tsx:81
— not affected: drilled period-label, series-name and formatted-value escapes
are present; removing each separately fails the drilled regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:230
— not affected: grouped-category escape is present; removing it fails the
Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:234
— not affected: translated Range and containing-band escapes are present;
removing each separately fails the Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:237
— not affected: metric-label and measure-value escapes are present; removing
each separately fails the Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:266
— not affected: range-label and range-value escapes are present; removing each
separately fails the Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:283
— not affected: marker-label and marker-value escapes are present; removing
each separately fails the Bullet regression.
- superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:1046 —
not affected: the shared encodeHTML wrapper is unchanged.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -424,3 +424,38 @@ test('keeps exact-case dimensions and uses per-index
fallbacks for short labels'
.map(series => series.markLine?.data[0].label.formatter()),
).toEqual(['stretch', '260.0']);
});
+
+test('escapes literal comparison text in every Bullet tooltip', () => {
+ const label = 'Users <30 & <img src=x>';
+ const metric = 'Total <3';
Review Comment:
Fixed in 214f0b4ce2781a32c33909c62154e017253e8670.
I changed the metric to `'<b>Total <3</b>'` in the regression test 'escapes
literal comparison text in every Bullet tooltip'. Its literal text must
survive, and each tooltip must have exactly one structural `<b>`. I reproduced
the gap: the original test passes without `escapeHtml(metricLabel)`; the
strengthened test fails that same mutation and passes with the escape restored.
The sibling sweep also exercises markup in formatted numbers, translated
range text and Rose period labels. All 18 individual escape-removal mutations
fail DOM assertions. Restored-source Jest: 3 suites, 32 tests passed (`CI=true
npm run test -- --runInBand
plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts
plugins/plugin-chart-echarts/test/Rose/transformProps.test.ts
plugins/plugin-chart-echarts/test/Rose/EchartsRose.test.tsx --coverage
--coverageReporters=json`).
Siblings checked:
-
superset-frontend/plugins/plugin-chart-echarts/test/Rose/transformProps.test.ts:207
— fixed in 214f0b4ce2781a32c33909c62154e017253e8670
-
superset-frontend/plugins/plugin-chart-echarts/test/Rose/EchartsRose.test.tsx:190
— fixed in 214f0b4ce2781a32c33909c62154e017253e8670
-
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:442
— fixed in 214f0b4ce2781a32c33909c62154e017253e8670
-
superset-frontend/plugins/plugin-chart-echarts/src/Rose/transformProps.ts:136 —
not affected: series-name and formatted-value escapes are present; removing
each separately fails the Rose tooltip regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Rose/transformProps.ts:141 —
not affected: rich period-label escape is present; removing it fails only the
rich case.
-
superset-frontend/plugins/plugin-chart-echarts/src/Rose/transformProps.ts:183 —
not affected: plain period-label, series-name and formatted-value escapes are
present; removing each separately fails the plain case.
- superset-frontend/plugins/plugin-chart-echarts/src/Rose/EchartsRose.tsx:81
— not affected: drilled period-label, series-name and formatted-value escapes
are present; removing each separately fails the drilled regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:230
— not affected: grouped-category escape is present; removing it fails the
Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:234
— not affected: translated Range and containing-band escapes are present;
removing each separately fails the Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:237
— not affected: metric-label and measure-value escapes are present; removing
each separately fails the Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:266
— not affected: range-label and range-value escapes are present; removing each
separately fails the Bullet regression.
-
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:283
— not affected: marker-label and marker-value escapes are present; removing
each separately fails the Bullet regression.
- superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts:1046 —
not affected: the shared encodeHTML wrapper is unchanged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
