The Receipt Printer tab stops throwing on every click - #773
Merged
Merged
Conversation
Owner: "receipt printer previously its multi selct. have you changed
anything?"
Nothing had. The multi-select was there and working - #rpPrinterList,
checkboxes, saved as `receipt_printers`. But beside it the same file carried a
SECOND printer picker that had never worked at all.
refreshPrinters, selectPrinter, printToSelected and printToDefault wrote to
#printerStatus, #printerList, #printBtn, #printContent and #copies. Not one of
those elements has ever existed here: `git log --follow -S 'id="printerList"'`
finds no commit that added or removed them, including before the move into
src/.
switchTab called it on every receipt-tab click, and refreshPrinters did
`statusDiv.classList.remove('hidden')` on null OUTSIDE its own try block - so it
threw, unhandled. The throw came BEFORE `printers` was assigned, so the
`printers.length === 0` guard never went false and it threw again on every
single click, for the life of the file. Two of the four functions were never
called by anything at all.
Also removes a listener relabelling #rpDefaultOption, the "default printer"
entry of the old dropdown. Guarded by `if (opt)`, so it never threw; it simply
stopped doing anything when the checkbox list replaced the dropdown, and nothing
said so. OS_DEFAULT_LABEL stays - the warning about sending a receipt to the
system default still uses it.
WHY NOTHING CAUGHT THIS
tests/tools/dead-selectors.js exists for exactly this and could not see it: it
matches jQuery `$('#id')` only, and only inside .js under frontend/.../modules/.
The shell uses getElementById inside HTML, which is neither - so src/ was never
scanned at all, and the tool reported "All good" for the life of the fault.
So tests/tools/dead-shell-ids.js, which can be exact where the frontend one
cannot. The shell's pages are self-contained, markup and script in one file, so
the question is not "does anything anywhere create this id" but "does THIS
file" - no cross-file guessing and no verify-before-deleting caveat. Run against
the pre-fix file it reports all six; against this one, nothing.
The test proves that rather than asserting it: it builds a page with the same
shape and requires the check to find it. A guard nobody has watched fail is a
guard nobody should trust.
Desktop 2506 pass.
Contributor
|
Merged to Try it at https://develop.posnic.io, or run it yourself: git fetch origin develop && git checkout develop
npm install && npm --prefix api install
npm run dev # then http://localhost:3000When you have tested it, say what you did and what happened, and set Reporting that something is broken is as useful as fixing it. It is |
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.
No — the multi-select was there and working, and my changes to that file
added the word "printer" exactly once, inside a comment.
But checking turned up something worse beside it.
A second printer picker that had never worked
refreshPrinters,selectPrinter,printToSelectedandprintToDefaultwroteto five elements that have never existed in this file:
git log --follow -S 'id="printerList"'finds no commit that added or removedthem, including before the move into
src/. They were never there.switchTabcalled it on every receipt-tab click:and
refreshPrintersdidstatusDiv.classList.remove('hidden')on null,outside its own try block. So it threw, unhandled, into
switchTab. And thethrow came before
printerswas assigned, so the guard never went false — itthrew again on every single click, for the life of the file. Two of the four
functions were never called by anything at all.
Also removes a listener relabelling
#rpDefaultOption, the "default printer"entry of the old dropdown. Guarded by
if (opt), so it never threw — it justquietly stopped doing anything when the checkbox list replaced the dropdown.
OS_DEFAULT_LABELstays; the warning about sending a receipt to the systemdefault still uses it.
The multi-select is untouched:
#rpPrinterList,_collectTargets,receipt_printerssaved as a list, and "Tick at least one printer" stillrefusing an empty save. Tests pin all four.
Why nothing caught it
tests/tools/dead-selectors.jsexists for exactly this class of fault andcould not see it. It matches jQuery
$('#id')only, and only inside.jsfiles under
frontend/.../modules/. The shell usesgetElementByIdinside HTML,which is neither — so
src/was never scanned at all, and the tool reported"All good" throughout.
So:
tests/tools/dead-shell-ids.js, which can be exact where the frontendone cannot. The shell's pages are self-contained — markup and script in one file
— so the question is not "does anything anywhere create this id" but "does this
file". No cross-file guessing, no verify-before-deleting caveat.
The test proves that rather than asserting it: it builds a page with the same
shape and requires the check to find it. A guard nobody has watched fail is a
guard nobody should trust.
Desktop 2506 pass / 0 fail.