Fix: MB-18 visual label editor silently destroys templates it can't fully r…

[ZaynSaul]

Summary

Changing any label setting via the visual editor could overwrite/destroy a customer's
entire existing label configuration, instead of applying only the specific change made.

Root Cause

The visual label editor doesn't patch individual settings — every "Save" click rebuilds the
label's entire HTML from scratch (generateLabelTemplate()) using a narrow, fixed set of
fields (columns, rows, margins, a handful of checkboxes, up to 5 custom text lines). Its
counterpart, parseLabelTemplate(), can only recognize that exact same narrow shape when
reading a template back in. Anything outside it — custom CSS like transform: rotate(),
non-default dimensions, extra markup — is invisible to the parser, and is silently thrown
away the moment the visual editor regenerates and saves.

This wasn't a rare edge case: every label template Saldi ships (Brother 22606, Dymo 11354,
both "mit salg" variants) uses CSS the parser doesn't understand. Opening any of those in
the visual editor and saving any single setting — even something as trivial as text length —
would silently wipe out the real layout and replace it with generic filler.

What are the changes about?

• Added labelTemplateEditableVisually($labelText) — runs the template through
  parseLabelTemplate() → generateLabelTemplate() and checks whether that reproduces the
  original exactly. If it doesn't, the template has content the visual editor can't safely
  represent.
• The editor page (labels()) now forces raw-HTML mode, with an explanation, for any label
  that fails this check, instead of offering the destructive visual editor; "Switch to
  Visual Editor" is hidden for unsafe labels.
• The $saveLabel POST handler (systemdata/diverse.php) enforces the same check
  server-side — a save is refused, not silently destructive, whenever the label's current
  template isn't visual-editor-safe. This is the authoritative guard; the UI change alone
  couldn't be trusted to block every path.

Files Changed

• systemdata/sys_div_func.php — new labelTemplateEditableVisually() function; labels()
  now forces raw-HTML mode and hides "Switch to Visual Editor" for unsafe labels
• systemdata/diverse.php — $saveLabel POST handler now checks the guard before
  regenerating/saving
• tests/characterization/systemdata/LabelTemplateEditableVisuallyCharacterizationTest.test.php
  (new) — 8 tests

How to Test

1. Open each of the 4 shipped import templates (Brother 22606, Dymo 11354, both "mit salg"
   variants) in the label editor — verify each is forced into raw-HTML mode with an
   explanation, and "Switch to Visual Editor" is not offered.
2. Attempt to save via the visual editor path against one of those unsafe templates directly
   (bypassing the UI, e.g. via the POST handler) — verify the save is refused server-side and
   the template is preserved byte-for-byte.
3. Create/open a genuinely visual-editor-built label (safe template) and change a setting —
   verify it still saves normally, confirming the guard doesn't over-block legitimate use.
4. Open an empty label — verify it's correctly flagged safe.
5. Run LabelTemplateEditableVisuallyCharacterizationTest.test.php and confirm all 8 cases
   pass.
6. Reproduce the original destructive scenario against the pre-fix code (stash the fix,
   seed a real imported Brother template, simulate a "change one field" save) and confirm it
   destroys the label (rotate gone, real 50mm width replaced by generic 38.1mm) — proving
   the test suite actually pins this bug.
7. Restore the fix and re-run the same scenario — confirm the save is refused and the
   template is preserved exactly.

Acceptance Criteria

#	Scenario	Expected Result
1	All 4 shipped import templates	Correctly flagged unsafe for visual editing
2	Visual-editor-native label	Correctly flagged safe
3	Empty label	Correctly flagged safe
4	Save attempt against an unsafe template	Refused server-side, zero data loss, template preserved byte-for-byte
5	Save attempt against a safe (visual-editor-native) template	Still saves correctly

Verification

• Stashed the fix, seeded a real imported Brother template, simulated a "change one field"
  save — confirmed it genuinely destroyed the label (rotate gone, real 50mm width replaced
  by generic 38.1mm).
• Restored the fix, reran the same scenario — refused, template preserved byte-for-byte.
• Confirmed a genuinely visual-editor-built label still saves normally (guard doesn't
  over-block).

Have you checked the following?

• I have updated relevant documentation at https://github.com/DANOSOFT/saldi/wiki
• I have checked that i am not linking to any external resources such as external style- or JavaScript-files, that could compromise stability
• I have read and understood the developer guidelines
  https://docs.google.com/document/d/1GOmomtvKf21OV2VWNOIPweDi4gJHpMCK5qXzrPrypUc/edit?usp=sharing

[coderabbitai]

Warning

Review limit reached

Next included review available in 42 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0e6b3fee-b1f5-45c8-8aea-12ba5948dcb9

📥 Commits

Reviewing files that changed from the base of the PR and between 14e8ec4 and 5b53672.

📒 Files selected for processing (3)

• systemdata/diverse.php
• systemdata/sys_div_func.php
• tests/characterization/systemdata/LabelTemplateEditableVisuallyCharacterizationTest.test.php

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[SirRolin]

Performed test:
Opened raw html editor.
Added a line.
Saved.
It correctly blocked me from accessing visual editor.
Deleted everything in the template.
Saved. -> it regenerated the default template.
It incorrectly blocks me from accessing the visual editor.

[ZaynSaul]

@SirRolin Fixed in 8065a5f — thanks for the repro steps, they pinpointed it exactly.

Root cause: in sys_div_func.php's labels(), when a 'Standard' label's stored text is empty, a hardcoded placeholder template is substituted in for display in the raw-HTML textarea (it still uses the older $beskrivelse/$pris variable names, predating generateLabelTemplate()'s $minbeskrivelse/$minpris). The new MB-18 guard was running labelTemplateEditableVisually() on that placeholder instead of on the actual stored (empty) text, so it always evaluated to "not visually editable" for an emptied-out label — even though there's nothing there to lose.

Fix captures the stored text before the placeholder substitution and guards on that instead, so an emptied 'Standard' label correctly regains access to the visual editor. Added a regression test (testTheEmptyStandardDisplayPlaceholderIsNotItselfVisuallyEditable) pinning the placeholder-vs-stored-text distinction that caused this.

Also swapped the hardcoded Postgres credentials in LabelTemplateEditableVisuallyCharacterizationTest for the gitignored connect.php convention (935dd29) — same pattern as the fix on #507.

Could you re-test the same repro when you get a chance?

[ZaynSaul]

@SirRolin following up — could you re-test when you get a chance? This is the top of my queue for Monday's deadline (MB-17/MB-18 both hang on it).

To directly re-verify your repro: raw HTML editor → delete everything → save. It now correctly returns to the visual editor instead of staying locked in raw mode. Added a regression test pinning exactly that (testTheEmptyStandardDisplayPlaceholderIsNotItselfVisuallyEditable), plus re-ran the full suite live against real DB rows on both the pre-fix and post-fix code to confirm the before/after behavior.

Also cleaned up a stray if_isset() warning in the test helper itself (same calling-convention bug as a couple other tickets this week) — suite runs with zero warnings now.

No other changes since your review — just the targeted fix for what you found.