mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
fix: an unnamed language code took down the whole Zim manage page
/zim/manage rendered a white screen with `Cannot read properties of undefined
(reading 'toLowerCase')` repeated once per catalog row.
The language dropdown is built as `{value: code, label: iso_639_codes[code]}`.
That table covers ISO 639-1 and 639-2; the Kiwix catalog also carries 639-3
codes and locale variants -- `ami`, `ary`, `arz`, `be-tarask` -- and on the
running instance 89 of the codes in the catalog are absent from its 672-entry
table. Every one of those produced `label: undefined`, and a `searchable`
Mantine Select calls `option.label.toLowerCase()` while building its dropdown on
MOUNT, before anyone opens it. So a single unnamed language anywhere in the
catalog took the page with it, and Wikipedia alone carries dozens.
The option falls back to the code. Not to a blank: a user choosing which
language to subscribe to has to tell one unnamed language from another, and the
code is the only thing known about it. `iso_639_codes` is read defensively too
-- ManageZim initialises it to null and fills it from the API, so the first
render of every row happens with no table at all.
The other two lookup-derived labels in the app, in ControllerPage and Configs,
already fall back this way. Zim was the only one that did not.
Four cases, written first and confirmed to fail with the reported TypeError
rather than for some other reason -- the first version failed on "Table
component was not found in the tree", which would have proved nothing, so the
row is now given the table Mantine requires. One of the four is the inverse: a
code the table DOES name must still render its name, so the fallback cannot
quietly replace every label.
1128 jest / 63 suites, clean npm run build. Confirmed on the running app: the
page loads with 32 language dropdowns and no error overlay.
This commit is contained in:
parent
a17c1e526c
commit
a31a0686b3
2 changed files with 93 additions and 2 deletions
|
|
@ -292,7 +292,7 @@ const ViewerMessage = () => {
|
|||
</HandPointMessage>
|
||||
}
|
||||
|
||||
const ZimCatalogItemRow = ({item, subscriptions, iso_639_codes, fetchSubscriptions}) => {
|
||||
export const ZimCatalogItemRow = ({item, subscriptions, iso_639_codes, fetchSubscriptions}) => {
|
||||
const {name, languages, size} = item;
|
||||
const subscription = name in subscriptions ? subscriptions[name] : null;
|
||||
const subscriptionLanguage = subscription ? subscription['language'] : 'en';
|
||||
|
|
@ -328,8 +328,24 @@ const ZimCatalogItemRow = ({item, subscriptions, iso_639_codes, fetchSubscriptio
|
|||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* The code itself when the API's table has no name for it.
|
||||
*
|
||||
* `iso_639_codes` covers 639-1 and 639-2; the Kiwix catalog also carries 639-3 codes and
|
||||
* locale variants -- `ami`, `ary`, `arz`, `be-tarask` -- and 89 of the codes in the
|
||||
* catalog on a live WROLPi are absent from its 672-entry table. Each of those produced
|
||||
* `label: undefined`, and a `searchable` Mantine Select calls `option.label.toLowerCase()`
|
||||
* while building its dropdown on mount, before anyone opens it. One unnamed language
|
||||
* anywhere in the catalog therefore took the whole /zim/manage page down.
|
||||
*
|
||||
* The code rather than a blank: a user choosing which language to subscribe to has to be
|
||||
* able to tell one unnamed language from another, and the code is all we know.
|
||||
*
|
||||
* `iso_639_codes` is also null on first render -- ManageZim initialises it that way and
|
||||
* fills it from the API -- so it is read defensively too.
|
||||
*/
|
||||
const languageOptions = languages.map(i => {
|
||||
return {value: i, label: iso_639_codes[i]}
|
||||
return {value: i, label: (iso_639_codes && iso_639_codes[i]) || i}
|
||||
});
|
||||
const languageDropdown = <Select
|
||||
searchable
|
||||
|
|
|
|||
75
app/src/components/Zim.test.js
Normal file
75
app/src/components/Zim.test.js
Normal file
|
|
@ -0,0 +1,75 @@
|
|||
import React from 'react';
|
||||
import {render, screen} from '../test-utils';
|
||||
import {ZimCatalogItemRow} from './Zim';
|
||||
import {Table} from './ui';
|
||||
|
||||
/*
|
||||
* The Zim catalog's language dropdown.
|
||||
*
|
||||
* `/zim/manage` crashed the whole page -- a white screen with
|
||||
* `Cannot read properties of undefined (reading 'toLowerCase')` repeated for every row. The
|
||||
* dropdown is built as `{value: code, label: iso_639_codes[code]}`, and the catalog carries
|
||||
* language codes the API's table has no name for: 89 of them against a 672-entry table,
|
||||
* measured from the running instance. `ami`, `ary`, `arz`, `be-tarask` and the rest are
|
||||
* ISO 639-3 codes and locale variants; the table is 639-1/639-2.
|
||||
*
|
||||
* A missing name gave the option `label: undefined`, and a `searchable` Mantine Select runs
|
||||
* `option.label.toLowerCase()` while building its dropdown -- on mount, before anyone opens
|
||||
* it. So one unnamed language anywhere in the catalog took the page down.
|
||||
*/
|
||||
|
||||
const item = (languages) => ({name: 'Wikipedia (mini)', languages, size: 1024});
|
||||
|
||||
// The row renders `<Table.Row>`, which Mantine refuses to mount without a table around it --
|
||||
// so the row has to be given one, or the spec fails before it reaches the dropdown at all.
|
||||
const renderRow = (languages, iso_639_codes) => render(
|
||||
<Table>
|
||||
<Table.Body>
|
||||
<ZimCatalogItemRow item={item(languages)} subscriptions={[]}
|
||||
iso_639_codes={iso_639_codes}/>
|
||||
</Table.Body>
|
||||
</Table>);
|
||||
|
||||
// A deliberately small stand-in for the API's table: `eng` is named, `ami` is not. That is
|
||||
// the whole shape of the bug.
|
||||
const codes = {eng: 'English', fra: 'French'};
|
||||
|
||||
describe('the Zim catalog language dropdown', () => {
|
||||
it('renders a language the code table has no name for', () => {
|
||||
// `ami` is real -- it is in the catalog on a live WROLPi and absent from the table.
|
||||
renderRow(['eng', 'ami'], codes);
|
||||
|
||||
expect(screen.getByPlaceholderText('Language')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('falls back to the code itself, so the option is still identifiable', () => {
|
||||
/*
|
||||
* Not a blank entry: a user subscribing to a Zim has to be able to tell one unnamed
|
||||
* language from another, and the code is the only thing we know about it. Mantine
|
||||
* renders the selected option's label in the input.
|
||||
*/
|
||||
renderRow(['ami'], codes);
|
||||
const input = screen.getByPlaceholderText('Language');
|
||||
|
||||
// Mantine keeps the options in the DOM for a closed dropdown's combobox; the label
|
||||
// is what a user reads when they open it.
|
||||
expect(input).toBeInTheDocument();
|
||||
expect(document.body.textContent).toContain('ami');
|
||||
});
|
||||
|
||||
it('still prefers the real name when the table has one', () => {
|
||||
// The inverse: falling back must not replace names that exist.
|
||||
renderRow(['eng'], codes);
|
||||
|
||||
expect(document.body.textContent).toContain('English');
|
||||
expect(document.body.textContent).not.toContain('eng<');
|
||||
});
|
||||
|
||||
it('survives a table that never arrived', () => {
|
||||
// ManageZim initialises `iso_639_codes: null` and fills it from the API, so the first
|
||||
// render of every row happens with no table at all.
|
||||
renderRow(['eng', 'ami'], null);
|
||||
|
||||
expect(screen.getByPlaceholderText('Language')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue