feat(text): say so when a text parses into nothing (#278) - #282
Open
HugoFara wants to merge 1 commit into
Open
Conversation
A language whose word characters do not match the script of its texts does not fail. It parses successfully into nothing: the text saves, opens, and renders every character, and not one of them can be clicked, looked up or learned. TokenPersistence::save() takes the empty token list and returns silently, so a Chinese text on a Latin language stores a row, a sentence, and zero text items. From the outside that is indistinguishable from an ordinary text that has gone inert, which is what the reporter met and had no way to diagnose. ParseCoverage is the one place that decides a parse came out empty, so the reading view, the check-text page and the API agree on when to speak up. The test is a word-to-character ratio, not a plain zero: a non-Latin text usually matches a few stray tokens — a digit, a Latin fragment — and a zero test would let those through. The floor is one word per fifty characters, well under the one per six of Latin prose or the one per one of a character-split script, and it only applies to texts long enough to judge. Three surfaces, all of them ones the reader already reaches: - the reading view, where the confusion actually happens, via a parseWarning on the payload that already feeds it; - the check-text page, whose whole purpose is to answer "did my parsing work?" and which until now answered a failed parse with an empty list; - ParseText::execute(), for anything reading the API. Each says what happened and links to the language's settings. The banner is built through the DOM rather than by concatenation: the renderer's escapeHtml() leaves quotes alone, which is fine for text but would let an attribute value break out of the attribute it sits in. Verified against a real database: Chinese on Latin defaults warning=no_words banner shown Chinese configured right warning=ok no banner ordinary English warning=ok no banner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The backstop for #278, independent of #281.
The silent failure
A language whose Word Characters setting doesn't match the script of its texts does not fail. It parses successfully into nothing:
TokenPersistence::save()hasif (empty($tokens)) return;and returns without a word. The text saves, opens, and renders every character — and refuses to respond to any click. No exception, no log line, no message. From the outside it is indistinguishable from an ordinary text that has gone inert, which is exactly what the reporter met, and why their issue says "I don't actually know what the expected behavior is".One rule, three surfaces
ParseCoverageis the single place that decides a parse came out empty, so the reading view, the check-text page and the API agree on when to speak up.The test is a word-to-character ratio, not a plain zero. A non-Latin text on a Latin language usually matches a few stray tokens — a digit, a Latin fragment — and a zero test lets those through while the text is still unreadable. The floor is one word per fifty characters, well under the ~1-per-6 of Latin prose or the ~1-per-1 of a character-split script, and it only applies to texts long enough to judge (short titles and captions are exempt).
Surfaces, all ones the reader already reaches:
parseWarningon the payload that already feeds itParseText::execute()warningfield for anything reading the APIEach says what happened and links straight to
/languages/{id}/edit.Verified against a real database
The banner reads:
Notes for review
text_renderer.ts'sescapeHtml()usestextContent/innerHTML, which does not escape quotes. That's fine for text nodes but would let an attribute value break out of its attribute, sohrefgoes throughsetAttribute. A test covers it.TextReadingService::mainWordLoop()is dead code — no callers. Its query chainsselect()/selectRaw(), which overwrite rather than accumulate, so it selects only the last five columns and reads six undefined keys. Not touched here; worth a separate look, along with whether the method should exist at all.Checks
Psalm 0 errors · PHPCS 0 errors, 0 warnings · PHPUnit 9109 pass (11 new) · Vitest 4330 pass (4 new) · tsc and ESLint clean · assets rebuilt.
One existing frontend test mocked
text_rendererexhaustively and needed the new export added to its mock.