From 25c0c16461ab267fbee4da1402f16772f49501be Mon Sep 17 00:00:00 2001 From: Anthony Stirling <77850077+Frooodle@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:06:24 +0100 Subject: [PATCH] fix(formfill,saas,saml): follow-ups from the merged PR review --- .github/workflows/backend-build.yml | 8 +- ...mSaml2ResponseAuthenticationConverter.java | 10 ++- .../util/WorkflowMapperWetSignatureTest.java | 76 +++++++++++++++++++ .../ai/controller/AiCreateController.java | 6 +- .../AiCreateInternalController.java | 6 +- .../service/StripeUsageReportingService.java | 4 +- .../saas/legal/LegalDocumentRegistry.java | 9 +-- .../payg/entitlement/EntitlementGuard.java | 9 ++- .../api/ProcurementController.java | 4 +- .../procurement/legal/AgreementAssembler.java | 4 +- .../KeygenEnterpriseLicenseService.java | 6 +- .../service/ProcurementService.java | 6 +- .../entitlement/EntitlementGuardTest.java | 6 +- .../src/core/tools/formFill/FieldInput.tsx | 5 +- .../core/tools/formFill/FormFieldOverlay.tsx | 6 +- .../src/core/tools/formFill/FormSaveBar.tsx | 10 ++- .../core/tools/formFill/checkboxState.test.ts | 70 +++++++++++++++++ .../src/core/tools/formFill/checkboxState.ts | 60 +++++++++++++++ .../formFill/providers/PdfiumFormProvider.ts | 36 ++++++++- 19 files changed, 294 insertions(+), 47 deletions(-) create mode 100644 app/proprietary/src/test/java/stirling/software/proprietary/workflow/util/WorkflowMapperWetSignatureTest.java create mode 100644 frontend/editor/src/core/tools/formFill/checkboxState.test.ts create mode 100644 frontend/editor/src/core/tools/formFill/checkboxState.ts diff --git a/.github/workflows/backend-build.yml b/.github/workflows/backend-build.yml index 09c6bbc9a8..95d7729c8c 100644 --- a/.github/workflows/backend-build.yml +++ b/.github/workflows/backend-build.yml @@ -54,10 +54,10 @@ jobs: - name: Install Task uses: go-task/setup-task@01a4adf9db2d14c1de7a560f09170b6e0df736aa # v2.1.0 - name: Check Java formatting (Spotless) - # Runs once per matrix combination - pick the cheapest leg - # (core - no proprietary, no saas) so we don't wait for the - # heavier flavors just to fail formatting. - if: matrix.jdk-version == 25 && matrix.flavor == 'core' + # Runs once per matrix combination, on the saas leg because that is the + # only flavor whose settings.gradle includes every module. On the core + # leg :proprietary and :saas are absent, so their Java is never checked. + if: matrix.jdk-version == 25 && matrix.flavor == 'saas' id: spotless-check run: task backend:format:check continue-on-error: true diff --git a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java index 96dcdecd03..0466b1eaad 100644 --- a/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java +++ b/app/proprietary/src/main/java/stirling/software/proprietary/security/saml2/CustomSaml2ResponseAuthenticationConverter.java @@ -14,8 +14,11 @@ import org.opensaml.saml.saml2.core.AuthnStatement; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.core.convert.converter.Converter; import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.saml2.core.Saml2Error; +import org.springframework.security.saml2.core.Saml2ErrorCodes; import org.springframework.security.saml2.provider.service.authentication.OpenSaml5AuthenticationProvider.ResponseToken; import org.springframework.security.saml2.provider.service.authentication.Saml2Authentication; +import org.springframework.security.saml2.provider.service.authentication.Saml2AuthenticationException; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -64,7 +67,12 @@ public class CustomSaml2ResponseAuthenticationConverter List assertions = responseToken.getResponse().getAssertions(); if (assertions == null || assertions.isEmpty()) { log.error("SAML response contains no assertions"); - return null; + // Returning null makes ProviderManager report "no provider found" and loses the + // SAML error code the failure handler renders. + throw new Saml2AuthenticationException( + new Saml2Error( + Saml2ErrorCodes.MALFORMED_RESPONSE_DATA, + "No assertions found in response.")); } Assertion assertion = assertions.getFirst(); Map> attributes = extractAttributes(assertion); diff --git a/app/proprietary/src/test/java/stirling/software/proprietary/workflow/util/WorkflowMapperWetSignatureTest.java b/app/proprietary/src/test/java/stirling/software/proprietary/workflow/util/WorkflowMapperWetSignatureTest.java new file mode 100644 index 0000000000..bd990d6123 --- /dev/null +++ b/app/proprietary/src/test/java/stirling/software/proprietary/workflow/util/WorkflowMapperWetSignatureTest.java @@ -0,0 +1,76 @@ +package stirling.software.proprietary.workflow.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import stirling.software.proprietary.workflow.dto.ParticipantResponse; +import stirling.software.proprietary.workflow.model.WorkflowParticipant; + +import tools.jackson.databind.ObjectMapper; + +/** + * Locks the wet-signature extraction contract across the Jackson 2 to Jackson 3 move (#7444). + * + *

Jackson 3 defaults FAIL_ON_UNKNOWN_PROPERTIES to false, where Jackson 2 defaulted it to true. + * Under Jackson 2 an unrecognised key threw, and the catch-all in {@code extractWetSignatures} + * discarded every signature for that participant. These tests pin the current behaviour so a future + * mapper built with FAIL_ON_UNKNOWN_PROPERTIES re-enabled cannot silently reintroduce that loss. + */ +class WorkflowMapperWetSignatureTest { + + private final ObjectMapper objectMapper = new ObjectMapper(); + + private WorkflowParticipant participantWithMetadata(Map signature) { + Map metadata = new HashMap<>(); + metadata.put("wetSignatures", List.of(signature)); + + WorkflowParticipant participant = new WorkflowParticipant(); + participant.setId(7L); + participant.setParticipantMetadata(metadata); + return participant; + } + + private Map signature() { + Map signature = new HashMap<>(); + signature.put("type", "draw"); + signature.put("data", "data:image/png;base64,AAAA"); + signature.put("page", 2); + signature.put("x", 10.5); + signature.put("y", 20.5); + signature.put("width", 100.0); + signature.put("height", 40.0); + return signature; + } + + @Test + void extractsAKnownSignature() { + ParticipantResponse response = + WorkflowMapper.toParticipantResponse( + participantWithMetadata(signature()), objectMapper, false); + + assertNotNull(response); + assertEquals(1, response.getWetSignatures().size()); + assertEquals("draw", response.getWetSignatures().getFirst().getType()); + assertEquals(2, response.getWetSignatures().getFirst().getPage()); + } + + @Test + void keepsSignaturesWhenMetadataCarriesAnUnknownKey() { + Map signature = signature(); + signature.put("unknownKeyFromAnOlderClient", "ignored"); + + ParticipantResponse response = + WorkflowMapper.toParticipantResponse( + participantWithMetadata(signature), objectMapper, false); + + assertNotNull(response); + assertEquals(1, response.getWetSignatures().size()); + assertEquals("draw", response.getWetSignatures().getFirst().getType()); + } +} diff --git a/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateController.java b/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateController.java index 77a87a27bb..d56849ff7b 100644 --- a/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateController.java +++ b/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateController.java @@ -26,9 +26,6 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.servlet.mvc.method.annotation.StreamingResponseBody; -import tools.jackson.core.JacksonException; -import tools.jackson.databind.ObjectMapper; - import io.swagger.v3.oas.annotations.Hidden; import io.swagger.v3.oas.annotations.tags.Tag; @@ -53,6 +50,9 @@ import stirling.software.saas.payg.model.JobSource; import stirling.software.saas.payg.model.ProcessType; import stirling.software.saas.util.AuthenticationUtils; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.ObjectMapper; + @RestController @Profile("saas") @RequestMapping("/api/v1/ai/create") diff --git a/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateInternalController.java b/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateInternalController.java index 04b8302e7e..ea9ee64145 100644 --- a/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateInternalController.java +++ b/app/saas/src/main/java/stirling/software/saas/ai/controller/AiCreateInternalController.java @@ -14,9 +14,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.server.ResponseStatusException; -import tools.jackson.core.JacksonException; -import tools.jackson.databind.ObjectMapper; - import io.swagger.v3.oas.annotations.Hidden; import io.swagger.v3.oas.annotations.tags.Tag; @@ -29,6 +26,9 @@ import stirling.software.saas.ai.service.AiCreateSessionService; import stirling.software.saas.payg.cap.RequiresFeature; import stirling.software.saas.payg.model.FeatureGate; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.ObjectMapper; + @RestController @Profile("saas") @RequestMapping("/api/v1/ai/create/internal") diff --git a/app/saas/src/main/java/stirling/software/saas/billing/service/StripeUsageReportingService.java b/app/saas/src/main/java/stirling/software/saas/billing/service/StripeUsageReportingService.java index ae5d3d943a..f1870bb715 100644 --- a/app/saas/src/main/java/stirling/software/saas/billing/service/StripeUsageReportingService.java +++ b/app/saas/src/main/java/stirling/software/saas/billing/service/StripeUsageReportingService.java @@ -12,12 +12,12 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Profile; import org.springframework.stereotype.Service; -import tools.jackson.databind.ObjectMapper; - import lombok.extern.slf4j.Slf4j; import stirling.software.saas.config.SupabaseConfigurationProperties; +import tools.jackson.databind.ObjectMapper; + /** * Reports per-tenant overage to Stripe Billing Meters via the Supabase {@code meter-usage} Edge * Function. Only credits consumed above the free tier flow through {@link #reportUsageToStripe}. diff --git a/app/saas/src/main/java/stirling/software/saas/legal/LegalDocumentRegistry.java b/app/saas/src/main/java/stirling/software/saas/legal/LegalDocumentRegistry.java index 48dd6a1c3b..2111a52112 100644 --- a/app/saas/src/main/java/stirling/software/saas/legal/LegalDocumentRegistry.java +++ b/app/saas/src/main/java/stirling/software/saas/legal/LegalDocumentRegistry.java @@ -13,13 +13,13 @@ import java.util.regex.Pattern; import org.springframework.core.io.ClassPathResource; import org.springframework.stereotype.Service; -import tools.jackson.databind.JsonNode; -import tools.jackson.databind.ObjectMapper; - import jakarta.annotation.PostConstruct; import lombok.extern.slf4j.Slf4j; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectMapper; + /** * Loads the versioned legal-document registry from {@code legal/manifest.json} on startup and * serves document metadata + rendered markdown from the classpath. @@ -63,8 +63,7 @@ public class LegalDocumentRegistry { d.path("parts"), objectMapper .getTypeFactory() - .constructCollectionType( - List.class, String.class)); + .constructCollectionType(List.class, String.class)); documents.put( id, new LegalDocumentMeta( diff --git a/app/saas/src/main/java/stirling/software/saas/payg/entitlement/EntitlementGuard.java b/app/saas/src/main/java/stirling/software/saas/payg/entitlement/EntitlementGuard.java index 441ebf1220..52960934c7 100644 --- a/app/saas/src/main/java/stirling/software/saas/payg/entitlement/EntitlementGuard.java +++ b/app/saas/src/main/java/stirling/software/saas/payg/entitlement/EntitlementGuard.java @@ -20,8 +20,6 @@ import org.springframework.stereotype.Component; import org.springframework.web.method.HandlerMethod; import org.springframework.web.servlet.HandlerInterceptor; -import tools.jackson.databind.ObjectMapper; - import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.MeterRegistry; @@ -40,6 +38,9 @@ import stirling.software.saas.payg.cap.RequiresFeature; import stirling.software.saas.payg.model.FeatureGate; import stirling.software.saas.util.AuthenticationUtils; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.ObjectMapper; + /** * Hot-path entitlement check. Runs after {@code PaygChargeInterceptor} in the MVC chain and short- * circuits the request before any handler work happens when the team's snapshot is missing one of @@ -357,9 +358,11 @@ public class EntitlementGuard implements HandlerInterceptor { response.setHeader(HttpHeaders.CONTENT_LENGTH, Integer.toString(payload.length)); response.getOutputStream().write(payload); response.getOutputStream().flush(); - } catch (IOException e) { + } catch (IOException | JacksonException e) { // Container will fall back to its default error page — we did set the status code, // so the client still sees the right HTTP code even if the body fails to write. + // JacksonException is unchecked and not an IOException, so serialization failures + // would otherwise escape preHandle and turn a 402 into a 500. log.warn("EntitlementGuard write response body failed", e); errorsCounter.increment(); } diff --git a/app/saas/src/main/java/stirling/software/saas/procurement/api/ProcurementController.java b/app/saas/src/main/java/stirling/software/saas/procurement/api/ProcurementController.java index c08778bdbc..27ec2f1391 100644 --- a/app/saas/src/main/java/stirling/software/saas/procurement/api/ProcurementController.java +++ b/app/saas/src/main/java/stirling/software/saas/procurement/api/ProcurementController.java @@ -18,8 +18,6 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; -import tools.jackson.databind.ObjectMapper; - import io.swagger.v3.oas.annotations.Hidden; import jakarta.servlet.http.HttpServletRequest; @@ -43,6 +41,8 @@ import stirling.software.saas.procurement.pricing.QuoteLineItem; import stirling.software.saas.procurement.service.ProcurementService; import stirling.software.saas.util.AuthenticationUtils; +import tools.jackson.databind.ObjectMapper; + /** * The enterprise procurement journey for a linked team: read the deal snapshot, start/extend a * (mock-licensed) trial, build a server-priced quote, and accept it. Stripe checkout itself is a diff --git a/app/saas/src/main/java/stirling/software/saas/procurement/legal/AgreementAssembler.java b/app/saas/src/main/java/stirling/software/saas/procurement/legal/AgreementAssembler.java index e75f3cbb32..2555a82079 100644 --- a/app/saas/src/main/java/stirling/software/saas/procurement/legal/AgreementAssembler.java +++ b/app/saas/src/main/java/stirling/software/saas/procurement/legal/AgreementAssembler.java @@ -10,8 +10,6 @@ import java.util.Map; import org.springframework.stereotype.Service; -import tools.jackson.databind.ObjectMapper; - import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -22,6 +20,8 @@ import stirling.software.saas.procurement.pricing.ProcurementPricingService; import stirling.software.saas.procurement.pricing.QuoteConfig; import stirling.software.saas.procurement.pricing.QuoteLineItem; +import tools.jackson.databind.ObjectMapper; + /** * Builds the full Stirling Enterprise Agreement for a specific quote: the static MSA (Part A) and * DPA (Part C) from the {@link LegalDocumentRegistry}, with the dynamic Order Form (Part B) diff --git a/app/saas/src/main/java/stirling/software/saas/procurement/license/KeygenEnterpriseLicenseService.java b/app/saas/src/main/java/stirling/software/saas/procurement/license/KeygenEnterpriseLicenseService.java index a1c09a9cb9..22252d892b 100644 --- a/app/saas/src/main/java/stirling/software/saas/procurement/license/KeygenEnterpriseLicenseService.java +++ b/app/saas/src/main/java/stirling/software/saas/procurement/license/KeygenEnterpriseLicenseService.java @@ -16,13 +16,13 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.context.annotation.Profile; import org.springframework.stereotype.Service; -import tools.jackson.databind.JsonNode; -import tools.jackson.databind.ObjectMapper; - import lombok.extern.slf4j.Slf4j; import stirling.software.saas.procurement.config.KeygenConfigurationProperties; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectMapper; + /** * Real {@link EnterpriseLicenseService}: manages the team's enterprise licence directly against the * Keygen API (the "call Keygen from Java" direction), rather than via the Supabase edge functions diff --git a/app/saas/src/main/java/stirling/software/saas/procurement/service/ProcurementService.java b/app/saas/src/main/java/stirling/software/saas/procurement/service/ProcurementService.java index 26bfb733dd..388b95eef3 100644 --- a/app/saas/src/main/java/stirling/software/saas/procurement/service/ProcurementService.java +++ b/app/saas/src/main/java/stirling/software/saas/procurement/service/ProcurementService.java @@ -10,9 +10,6 @@ import org.springframework.context.annotation.Profile; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; -import tools.jackson.core.JacksonException; -import tools.jackson.databind.ObjectMapper; - import lombok.extern.slf4j.Slf4j; import stirling.software.common.model.enumeration.TeamRole; @@ -38,6 +35,9 @@ import stirling.software.saas.procurement.repository.ProcurementQuoteRepository; import stirling.software.saas.service.SaasTeamService; import stirling.software.saas.util.LogRedactionUtils; +import tools.jackson.core.JacksonException; +import tools.jackson.databind.ObjectMapper; + /** * Orchestrates a linked team's procurement journey: start a (mock-licensed) trial, build a * server-priced quote, and accept it. Stripe checkout itself lives in a Supabase edge function the diff --git a/app/saas/src/test/java/stirling/software/saas/payg/entitlement/EntitlementGuardTest.java b/app/saas/src/test/java/stirling/software/saas/payg/entitlement/EntitlementGuardTest.java index a9ca7642a6..dad953d61e 100644 --- a/app/saas/src/test/java/stirling/software/saas/payg/entitlement/EntitlementGuardTest.java +++ b/app/saas/src/test/java/stirling/software/saas/payg/entitlement/EntitlementGuardTest.java @@ -28,9 +28,6 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.web.method.HandlerMethod; -import tools.jackson.databind.JsonNode; -import tools.jackson.databind.ObjectMapper; - import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; @@ -45,6 +42,9 @@ import stirling.software.saas.payg.model.FeatureGate; import stirling.software.saas.payg.model.FeatureSet; import stirling.software.saas.security.EnhancedJwtAuthenticationToken; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.ObjectMapper; + /** * Pure-Mockito tests for {@link EntitlementGuard}. Covers the four decision-matrix cells: anonymous * billable → 401, anonymous manual → pass, authenticated FULL → pass, authenticated DEGRADED for a diff --git a/frontend/editor/src/core/tools/formFill/FieldInput.tsx b/frontend/editor/src/core/tools/formFill/FieldInput.tsx index 9a8943ea47..4a88e6a960 100644 --- a/frontend/editor/src/core/tools/formFill/FieldInput.tsx +++ b/frontend/editor/src/core/tools/formFill/FieldInput.tsx @@ -18,6 +18,7 @@ import { import { useTranslation } from "react-i18next"; import { useFieldValue } from "@app/tools/formFill/FormFillContext"; import type { FormField } from "@app/tools/formFill/types"; +import { isFieldChecked } from "@app/tools/formFill/checkboxState"; function FieldInputInner({ field, @@ -69,9 +70,7 @@ function FieldInputInner({ case "checkbox": { const exportVal = field.widgets && field.widgets[0]?.exportValue; - const isChecked = exportVal - ? value === exportVal || value === "Yes" - : !!value && value !== "Off"; + const isChecked = isFieldChecked(field.widgets, value); const onValue = exportVal || "Yes"; return ( diff --git a/frontend/editor/src/core/tools/formFill/checkboxState.test.ts b/frontend/editor/src/core/tools/formFill/checkboxState.test.ts new file mode 100644 index 0000000000..3fdc8915c6 --- /dev/null +++ b/frontend/editor/src/core/tools/formFill/checkboxState.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; + +import { + isFieldChecked, + isGenericOn, + isWidgetChecked, +} from "@app/tools/formFill/checkboxState"; + +describe("isWidgetChecked", () => { + it("ticks only the widget whose export value matches", () => { + expect(isWidgetChecked({ exportValue: "Red" }, "Red")).toBe(true); + expect(isWidgetChecked({ exportValue: "Blue" }, "Red")).toBe(false); + }); + + it("accepts the on-state spellings the backend accepts", () => { + for (const value of ["Yes", "true", "1", "on", "checked", "CHECKED"]) { + expect(isWidgetChecked({ exportValue: "Red" }, value)).toBe(true); + } + }); + + it("falls back to non-Off when the widget has no export value", () => { + expect(isWidgetChecked({ exportValue: null }, "Yes")).toBe(true); + expect(isWidgetChecked({ exportValue: null }, "Off")).toBe(false); + expect(isWidgetChecked({ exportValue: null }, "off")).toBe(false); + expect(isWidgetChecked({ exportValue: null }, "")).toBe(false); + }); + + it("treats a missing value as unchecked", () => { + expect(isWidgetChecked({ exportValue: "Red" }, undefined)).toBe(false); + expect(isWidgetChecked(undefined, null)).toBe(false); + }); +}); + +describe("isFieldChecked", () => { + const widgets = [{ exportValue: "Red" }, { exportValue: "Blue" }]; + + it("ticks when any kid matches, not only the first", () => { + expect(isFieldChecked(widgets, "Blue")).toBe(true); + expect(isFieldChecked(widgets, "Red")).toBe(true); + }); + + it("stays unticked for a state no kid carries", () => { + expect(isFieldChecked(widgets, "Green")).toBe(false); + expect(isFieldChecked(widgets, "Off")).toBe(false); + }); + + it("accepts legacy on-values against a field that has export values", () => { + expect(isFieldChecked(widgets, "true")).toBe(true); + }); + + it("falls back to non-Off when no kid declares an export value", () => { + expect(isFieldChecked([{ exportValue: null }], "Yes")).toBe(true); + expect(isFieldChecked([{ exportValue: null }], "Off")).toBe(false); + }); +}); + +describe("isGenericOn", () => { + it("is false for empty, whitespace and Off in any case", () => { + expect(isGenericOn("")).toBe(false); + expect(isGenericOn(" ")).toBe(false); + expect(isGenericOn("Off")).toBe(false); + expect(isGenericOn("OFF")).toBe(false); + expect(isGenericOn(null)).toBe(false); + }); + + it("is true for any other non-empty state", () => { + expect(isGenericOn("Yes")).toBe(true); + expect(isGenericOn("Red")).toBe(true); + }); +}); diff --git a/frontend/editor/src/core/tools/formFill/checkboxState.ts b/frontend/editor/src/core/tools/formFill/checkboxState.ts new file mode 100644 index 0000000000..89425aedc8 --- /dev/null +++ b/frontend/editor/src/core/tools/formFill/checkboxState.ts @@ -0,0 +1,60 @@ +/** + * One definition of "is this checkbox on", shared by the sidebar row, the page + * overlay and the PDFium save path. + * + * Must stay in sync with FormUtils.isChecked (app/common) — the backend accepts + * these spellings regardless of the widget's appearance state, so a UI that only + * accepted the widget's own export value would render unticked boxes that the + * backend still saves ticked. + */ +const LEGACY_ON_VALUES = new Set(["yes", "true", "1", "on", "checked"]); + +/** The PDF "off" appearance state. Compared case-insensitively, as the backend does. */ +const OFF_STATE = "off"; + +interface WidgetOnState { + exportValue?: string | null; +} + +function isLegacyOn(value: string): boolean { + return LEGACY_ON_VALUES.has(value.trim().toLowerCase()); +} + +/** True when a field value that names no export state still means "on". */ +export function isGenericOn(value: string | null | undefined): boolean { + if (!value) return false; + const trimmed = value.trim(); + return trimmed !== "" && trimmed.toLowerCase() !== OFF_STATE; +} + +/** + * Whether one widget renders ticked for the field's current value. + * + * A checkbox field's kid widgets can carry different on-states, so the answer is + * per widget: only the kid whose export value matches is ticked. + */ +export function isWidgetChecked( + widget: WidgetOnState | null | undefined, + value: string | null | undefined, +): boolean { + if (!value) return false; + const exportValue = widget?.exportValue; + if (exportValue) return value === exportValue || isLegacyOn(value); + return isGenericOn(value); +} + +/** + * Whether a field reads as ticked anywhere, for UI that has no widget in hand. + * + * Matches against every kid rather than the first: a field whose checked kid is + * not kid 0 would otherwise render unticked. + */ +export function isFieldChecked( + widgets: ReadonlyArray | null | undefined, + value: string | null | undefined, +): boolean { + if (!value) return false; + if (widgets?.some((w) => isWidgetChecked(w, value))) return true; + const hasExportValues = widgets?.some((w) => Boolean(w.exportValue)); + return hasExportValues ? isLegacyOn(value) : isGenericOn(value); +} diff --git a/frontend/editor/src/core/tools/formFill/providers/PdfiumFormProvider.ts b/frontend/editor/src/core/tools/formFill/providers/PdfiumFormProvider.ts index 471bba5e43..b83dd846ee 100644 --- a/frontend/editor/src/core/tools/formFill/providers/PdfiumFormProvider.ts +++ b/frontend/editor/src/core/tools/formFill/providers/PdfiumFormProvider.ts @@ -14,6 +14,7 @@ * for both providers. */ import { PDF_FORM_FIELD_TYPE } from "@app/services/pdfiumService"; +import type { WrappedPdfiumModule } from "@embedpdf/pdfium"; import { FPDF_ANNOT_WIDGET, FLAT_PRINT } from "@app/utils/pdfiumBitmapUtils"; import type { FormField, @@ -23,6 +24,26 @@ import type { } from "@app/tools/formFill/types"; import type { IFormDataProvider } from "@app/tools/formFill/providers/types"; import type { PDFDict } from "@cantoo/pdf-lib"; +import { isWidgetChecked } from "@app/tools/formFill/checkboxState"; + +/** Reads one widget annotation's /AP on-state name, or null when it has none. */ +function readAnnotExportValue( + m: WrappedPdfiumModule, + formEnvPtr: number, + annotPtr: number, +): string | null { + try { + const len = m.FPDFAnnot_GetFormFieldExportValue(formEnvPtr, annotPtr, 0, 0); + if (len <= 0) return null; + const buf = m.pdfium.wasmExports.malloc(len); + m.FPDFAnnot_GetFormFieldExportValue(formEnvPtr, annotPtr, buf, len); + const out = readUtf16(m, buf, len); + m.pdfium.wasmExports.free(buf); + return out || null; + } catch { + return null; + } +} interface PDFAcroField { dict: PDFDict; @@ -92,7 +113,8 @@ function toFormField( // Derive value string let value = f.value; if (type === "checkbox") { - value = f.isChecked ? f.widgets[0]?.exportValue || "Yes" : "Off"; + const checkedWidget = f.widgets.find((w) => w.isChecked) ?? f.widgets[0]; + value = f.isChecked ? checkedWidget?.exportValue || "Yes" : "Off"; } else if (type === "radio") { // Use widget index as the canonical radio value. // This avoids issues with duplicate exportValues across widgets @@ -615,7 +637,17 @@ export class PdfiumFormProvider implements IFormDataProvider { formEnvPtr, annotPtr, ); - const shouldBeChecked = value !== "" && value !== "Off"; + // Per widget, not per field: a checkbox field's kid widgets can + // carry different on-states, and only the matching kid is ticked. + const widgetExportValue = readAnnotExportValue( + m, + formEnvPtr, + annotPtr, + ); + const shouldBeChecked = isWidgetChecked( + { exportValue: widgetExportValue }, + value, + ); if (isCurrentlyChecked !== shouldBeChecked) { const ENTER_KEY = 13; m.FORM_SetFocusedAnnot(formEnvPtr, annotPtr);