Skip to content

Commit 1ab8e25

Browse files
prateek1497Muskan Guptavduamuskguptarismehta
authored
Merge Dev into master (#1894)
* Updating style selectors for theme editor * Updating style selectors further * Merge pull request #1872 from adobe/fix/FORMS-25246-fieldtype-containers feat(dialog): add fieldType hidden field to container component dialogs * fix(checkbox): guard against null uncheckedValue when JCR property is absent (#1892) When enableUncheckedValue=true but uncheckedValue is not stored in JCR (e.g. because the content push mechanism skipped an empty-string value), AbstractCheckboxImpl built an enum array with null as the second element. The AFB runtime calls .toString() on every enum value at init time, so enum: [".", null] caused TypeError: Cannot read properties of null (reading 'toString') in createFormInstance / RuleEngineWorker, crashing the form before any screen rendered. Fix: default safeUncheckedValue to "" when the JCR property is absent. Add a regression test with a fixture that omits uncheckedValue from JCR. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(af-core): FORMS-25463 expose cq:annotations parallel to fd:dor (#1883) * feat(af-core): FORMS-25463 expose cq:annotations parallel to fd:dor Add getCqAnnotations() on AbstractFormComponentImpl that reads the cq:annotations child resource (a sibling of fd:dorContainer on the component node) and exposes its child annotations as a per-annotation map keyed by node name. Each entry carries color, text, x, y, optional state/resolvedBy/resolvedAt and the JCR audit fields. Null fields are stripped so the serialised shape matches the JCR content exactly. Wire the call into getProperties() so the resulting map is placed under properties.cq:annotations -- parallel to fd:dor / fd:path / fd:associate -- not nested inside dorContainer. Returns null when no cq:annotations child exists; Jackson then omits the key, keeping the output non-breaking for forms without annotations. * fix(af-core): FORMS-25463 exclude cq:annotations from getCustomProperties getCustomProperties() reads all non-reserved JCR properties from the resource ValueMap and includes them as raw Strings. cq:annotations is not in the excluded-prefix list ("fd:", "jcr:", "sling:"), so a stale String-typed cq:annotations property on the node was leaking through and appearing as a double-serialised JSON string in the GET IC response. Adding an explicit key exclusion for CUSTOM_ANNOTATIONS_PROPERTY_WRAPPER ensures the property is handled exclusively by getCqAnnotations(), which returns it as Map<String,Object> and produces proper nested JSON. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(af-core): FORMS-25463 ic annotations in authoring crispr only * style(af-core): FORMS-25463 apply formatter to AbstractFormComponentImpl Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(af-core): FORMS-25463 ic annotations in authoring crispr only * fix(af-core): FORMS-25463 remove formatting changes, keep only cq:annotations feature * FORMS-25463: blocking publish of cq:annotations * FORMS-25463: adding tests * FORMS-25463: adding more tests for codecov * FORMS-25463: formatting fix * FORMS-25463: PR comments incorporate plus tests --------- Co-authored-by: Prateek Awasthi <prateekawast@adobe.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Muskan Gupta <muskgupta@Muskans-MacBook-Pro.local> Co-authored-by: Varun Dua <vdua@adobe.com> Co-authored-by: muskgupta <muskgupta@adobe.com> Co-authored-by: Rishi Mehta <69448117+rismehta@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Prateek Awasthi <prateekawast@adobe.com>
1 parent 4c9473f commit 1ab8e25

38 files changed

Lines changed: 1378 additions & 323 deletions

File tree

bundles/af-core/src/main/java/com/adobe/cq/forms/core/components/internal/form/ReservedProperties.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,8 @@ private ReservedProperties() {
176176

177177
public static final String FD_DRAFT_ID = "fd:draftId";
178178

179+
public static final String PN_CQ_ANNOTATIONS = "cq:annotations";
180+
179181
// Begin: Form submission related properties
180182
public static final String FD_SUBMIT_PROPERTIES = "fd:submit";
181183
public static final String PN_SUBMIT_ACTION_TYPE = "actionType";

bundles/af-core/src/main/java/com/adobe/cq/forms/core/components/util/AbstractCheckboxImpl.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,11 @@ public void initBaseCheckboxModel() {
4646
checkedValue = String.valueOf(getEnums()[0]);
4747
uncheckedValue = getEnums().length > 1 ? String.valueOf(getEnums()[1]) : null;
4848
}
49-
enums = (checkedValue != null) ? (Boolean.TRUE.equals(enableUncheckedValue)) ? new String[] { checkedValue, uncheckedValue }
49+
// Guard against null uncheckedValue: when enableUncheckedValue is true the JCR property
50+
// may be absent (e.g. because the push mechanism skipped an empty-string value), which
51+
// would put null into the enum array and crash the runtime on .toString() at init time.
52+
String safeUncheckedValue = uncheckedValue != null ? uncheckedValue : "";
53+
enums = (checkedValue != null) ? (Boolean.TRUE.equals(enableUncheckedValue)) ? new String[] { checkedValue, safeUncheckedValue }
5054
: new String[] { checkedValue } : null;
5155
}
5256

bundles/af-core/src/main/java/com/adobe/cq/forms/core/components/util/AbstractFormComponentImpl.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,8 @@ protected boolean getEditMode() {
314314

315315
public static final String CUSTOM_RULE_PROPERTY_WRAPPER = "fd:rules";
316316

317+
public static final String CUSTOM_ANNOTATIONS_PROPERTY_WRAPPER = "cq:annotations";
318+
317319
/**
318320
* Predicate to check if a map entry is non empty
319321
* return true if and only if
@@ -337,6 +339,10 @@ protected boolean getEditMode() {
337339
if (!getDorProperties().isEmpty()) {
338340
properties.put(CUSTOM_DOR_PROPERTY_WRAPPER, getDorProperties());
339341
}
342+
Map<String, Object> annotations = getCqAnnotations();
343+
if (annotations != null) {
344+
properties.put(CUSTOM_ANNOTATIONS_PROPERTY_WRAPPER, annotations);
345+
}
340346
properties.put(CUSTOM_JCR_PATH_PROPERTY_WRAPPER, getPath());
341347
if (isAuthorMode(request) || FormConstants.CHANNEL_PRINT.equals(this.channel)) {
342348
Map<String, Object> rulesProperties = getRulesProperties();
@@ -740,4 +746,33 @@ public Map<String, Object> getDorContainer() {
740746
return null;
741747
}
742748

749+
/**
750+
* Returns the reviewer annotations stored under the component's {@code cq:annotations} child resource,
751+
* keyed by annotation node name. Scoped to the print channel; returns null when no annotations exist
752+
* so callers can omit the property.
753+
*/
754+
@JsonIgnore
755+
private Map<String, Object> getCqAnnotations() {
756+
if (!FormConstants.CHANNEL_PRINT.equals(this.channel) || resource == null) {
757+
return null;
758+
}
759+
Resource annotationsResource = resource.getChild(CUSTOM_ANNOTATIONS_PROPERTY_WRAPPER);
760+
if (annotationsResource == null) {
761+
return null;
762+
}
763+
Map<String, Object> result = new LinkedHashMap<>();
764+
for (Resource child : annotationsResource.getChildren()) {
765+
ValueMap vm = child.getValueMap();
766+
Map<String, Object> ann = vm.entrySet().stream()
767+
.filter(e -> isAllowedType(e.getValue())
768+
&& !e.getKey().startsWith("jcr:")
769+
&& !e.getKey().startsWith("sling:"))
770+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> a, LinkedHashMap::new));
771+
if (!ann.isEmpty()) {
772+
result.put(child.getName(), ann);
773+
}
774+
}
775+
return result.isEmpty() ? null : result;
776+
}
777+
743778
}

bundles/af-core/src/test/java/com/adobe/cq/forms/core/components/internal/models/v1/form/CheckBoxImplTest.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ public class CheckBoxImplTest {
5858

5959
private static final String PATH_CHECKBOX_ENABLEUNCHECKEDOFF = CONTENT_ROOT + "/checkbox-enableUncheckedValueFalse";
6060
private static final String PATH_CHECKBOX_WITHOUT_FIELDTYPE = CONTENT_ROOT + "/checkbox-without-fieldtype";
61+
private static final String PATH_CHECKBOX_ENABLEUNCHECKED_MISSING = CONTENT_ROOT + "/checkbox-enableUncheckedValueMissingFromJcr";
6162
private final AemContext context = FormsCoreComponentTestContext.newAemContext();
6263

6364
@BeforeEach
@@ -350,6 +351,15 @@ void shouldOnlyHaveOnEnumIfEnableUncheckedValueOff() {
350351
assertArrayEquals(new String[] { "on" }, checkbox.getEnums());
351352
}
352353

354+
@Test
355+
void shouldUseEmptyStringWhenUncheckedValueMissingFromJcr() {
356+
// Regression: when enableUncheckedValue=true but the uncheckedValue JCR property is absent
357+
// (e.g. because the content push mechanism skipped an empty-string value), the enum array
358+
// must not contain null — null.toString() in the AFB runtime crashes the form at init time.
359+
CheckBox checkbox = getCheckBoxUnderTest(PATH_CHECKBOX_ENABLEUNCHECKED_MISSING);
360+
assertArrayEquals(new String[] { ".", "" }, checkbox.getEnums());
361+
}
362+
353363
private CheckBox getCheckBoxUnderTest(String resourcePath) {
354364
context.currentResource(resourcePath);
355365
MockSlingHttpServletRequest request = context.request();

bundles/af-core/src/test/java/com/adobe/cq/forms/core/components/util/AbstractFormComponentImplTest.java

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ public class AbstractFormComponentImplTest {
5757
private static final String PATH_COMPONENT_WITH_INVALID_XFA_SCRIPTS = CONTENT_ROOT + "/xfacomponentinvalid";
5858
private static final String PATH_COMPONENT_WITH_NO_XFA_SCRIPTS = CONTENT_ROOT + "/xfacomponentnone";
5959
private static final String PATH_COMPONENT_WITH_RULES = CONTENT_ROOT + "/textinputWithPrintRule";
60+
private static final String PATH_COMPONENT_WITH_PRINT_ANNOTATIONS = CONTENT_ROOT + "/textinputWithPrintAnnotations";
61+
private static final String PATH_COMPONENT_WITH_EMPTY_ANNOTATIONS = CONTENT_ROOT + "/textinputWithEmptyAnnotations";
6062
private static final String AF_PATH = "/content/forms/af/testAf";
6163
private static final String PAGE_PATH = "/content/testPage";
6264

@@ -273,6 +275,124 @@ public void testPrintChannelRule() {
273275
assertNotNull(formReadyRule);
274276
}
275277

278+
@Test
279+
public void testPrintChannelAnnotationsExcludedForWebChannel() {
280+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_PRINT_ANNOTATIONS);
281+
Utils.setInternalState(abstractFormComponentImpl, "channel", "web");
282+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
283+
assertNull(properties.get("cq:annotations"),
284+
"cq:annotations should not appear for non-print channel");
285+
}
286+
287+
@Test
288+
public void testPrintChannelAnnotationsIncluded() {
289+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_PRINT_ANNOTATIONS);
290+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
291+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
292+
Map<String, Object> annotations = (Map<String, Object>) properties.get("cq:annotations");
293+
assertNotNull(annotations, "cq:annotations should appear for print channel");
294+
Map<String, Object> annotation = (Map<String, Object>) annotations.get("annotation-1");
295+
assertNotNull(annotation);
296+
assertEquals("review note", annotation.get("text"));
297+
}
298+
299+
@Test
300+
public void testAnnotationPropertiesIncludeAllowedTypes() {
301+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_PRINT_ANNOTATIONS);
302+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
303+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
304+
Map<String, Object> annotations = (Map<String, Object>) properties.get("cq:annotations");
305+
assertNotNull(annotations);
306+
Map<String, Object> annotation = (Map<String, Object>) annotations.get("annotation-1");
307+
assertNotNull(annotation);
308+
// String and Long
309+
assertEquals("review note", annotation.get("text"));
310+
assertEquals(11L, annotation.get("x"));
311+
assertEquals(22L, annotation.get("y"));
312+
// Boolean
313+
assertEquals(Boolean.FALSE, annotation.get("resolved"));
314+
// String[]
315+
Object tags = annotation.get("tags");
316+
assertNotNull(tags);
317+
assertTrue(tags instanceof String[], "tags should be exported as String[]");
318+
assertEquals(2, ((String[]) tags).length);
319+
}
320+
321+
@Test
322+
public void testAnnotationPropertiesExcludeJcrAndSlingPrefixes() {
323+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_PRINT_ANNOTATIONS);
324+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
325+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
326+
Map<String, Object> annotations = (Map<String, Object>) properties.get("cq:annotations");
327+
assertNotNull(annotations);
328+
Map<String, Object> annotation = (Map<String, Object>) annotations.get("annotation-1");
329+
assertNotNull(annotation);
330+
assertNull(annotation.get("jcr:primaryType"), "jcr:primaryType must not leak into annotation output");
331+
assertNull(annotation.get("jcr:createdBy"), "jcr:createdBy must not leak into annotation output");
332+
assertNull(annotation.get("sling:resourceType"), "sling:* must not leak into annotation output");
333+
}
334+
335+
@Test
336+
public void testNoAnnotationsResource() {
337+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_NO_RULE);
338+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
339+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
340+
assertNull(properties.get("cq:annotations"),
341+
"cq:annotations should not appear when no annotations resource exists");
342+
}
343+
344+
@Test
345+
public void testAnnotationsAllChildEntriesFilteredOut() {
346+
// Child node contains only jcr:/sling: properties -> filtered map is empty,
347+
// child is skipped, and overall annotations map ends up empty -> getProperties
348+
// omits cq:annotations entirely. Covers the `!ann.isEmpty()` false branch and
349+
// the `result.isEmpty() ? null` true branch in getCqAnnotations().
350+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_EMPTY_ANNOTATIONS);
351+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
352+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
353+
assertNull(properties.get("cq:annotations"),
354+
"cq:annotations should be omitted when every child has only jcr:/sling: properties");
355+
}
356+
357+
@Test
358+
public void testAnnotationsWithNullResourceAndPrintChannel() {
359+
// channel=print but resource never set: must short-circuit and return null,
360+
// not NPE. Covers the `resource == null` branch in getCqAnnotations().
361+
AbstractFormComponentImpl abstractFormComponentImpl = new AbstractFormComponentImpl();
362+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
363+
Method getCqAnnotations = Utils.getPrivateMethod(AbstractFormComponentImpl.class, "getCqAnnotations");
364+
try {
365+
Object result = getCqAnnotations.invoke(abstractFormComponentImpl);
366+
assertNull(result, "getCqAnnotations should return null when resource is null");
367+
} catch (Exception e) {
368+
fail("getCqAnnotations should not throw when resource is null: " + e.getMessage());
369+
}
370+
}
371+
372+
@Test
373+
public void testAnnotationsAbsentInDefaultWebChannel() {
374+
// No channel set (defaults to non-print): annotations must not appear.
375+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_PRINT_ANNOTATIONS);
376+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
377+
assertNull(properties.get("cq:annotations"),
378+
"cq:annotations should not appear when channel is not print");
379+
}
380+
381+
@Test
382+
public void testMultipleAnnotations() {
383+
AbstractFormComponentImpl abstractFormComponentImpl = prepareTestClass(PATH_COMPONENT_WITH_PRINT_ANNOTATIONS);
384+
Utils.setInternalState(abstractFormComponentImpl, "channel", "print");
385+
Map<String, Object> properties = abstractFormComponentImpl.getProperties();
386+
Map<String, Object> annotations = (Map<String, Object>) properties.get("cq:annotations");
387+
assertNotNull(annotations);
388+
assertEquals(2, annotations.size(), "Should have 2 annotations");
389+
assertNotNull(annotations.get("annotation-1"));
390+
assertNotNull(annotations.get("annotation-2"));
391+
Map<String, Object> annotation2 = (Map<String, Object>) annotations.get("annotation-2");
392+
assertEquals("second review note", annotation2.get("text"));
393+
assertEquals("resolved", annotation2.get("state"));
394+
}
395+
276396
@Test
277397
public void testAssociateProperties() {
278398
Resource resource = Mockito.mock(Resource.class);
@@ -283,6 +403,7 @@ public void testAssociateProperties() {
283403
ValueMap valueMap = new MockValueMap(resource);
284404
Mockito.doReturn(valueMap).when(resource).getValueMap();
285405
Mockito.doReturn(null).when(resource).getChild("fd:dorContainer");
406+
Mockito.doReturn(null).when(resource).getChild("cq:annotations");
286407
Resource associateResource = Mockito.mock(Resource.class);
287408
Mockito.doReturn(associateResource).when(resource).getChild("fd:associate");
288409
Map<String, Object> properties = abstractFormComponentImpl.getProperties();

bundles/af-core/src/test/resources/form/checkbox/test-content.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@
9494
"jcr:primaryType": "nt:unstructured"
9595
}
9696
},
97+
"checkbox-enableUncheckedValueMissingFromJcr" : {
98+
"jcr:primaryType": "nt:unstructured",
99+
"sling:resourceType" : "core/fd/components/form/checkbox/v1/checkbox",
100+
"name" : "abc",
101+
"jcr:title" : "def",
102+
"type" : "string",
103+
"checkedValue": ".",
104+
"enableUncheckedValue": true,
105+
"fieldType": "checkbox"
106+
},
97107
"checkbox-without-fieldtype" : {
98108
"id": "checkbox-wo-fieldtype1",
99109
"jcr:primaryType": "nt:unstructured",

bundles/af-core/src/test/resources/form/componentswithrule/test-content.json

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,5 +65,53 @@
6565
"jcr:primaryType": "nt:unstructured",
6666
"fd:formReady": ["form Ready"]
6767
}
68+
},
69+
"textinputWithPrintAnnotations": {
70+
"jcr:primaryType": "nt:unstructured",
71+
"name": "nameWithAnnotations",
72+
"sling:resourceType": "forms-components-examples/components/form/textinput",
73+
"fieldType": "text-input",
74+
"fd:channel": "print",
75+
"cq:annotations": {
76+
"jcr:primaryType": "nt:unstructured",
77+
"annotation-1": {
78+
"jcr:primaryType": "nt:unstructured",
79+
"jcr:createdBy": "admin",
80+
"sling:resourceType": "cq/annotations",
81+
"text": "review note",
82+
"x": 11,
83+
"y": 22,
84+
"state": "open",
85+
"reviewedBy": "icreviewer",
86+
"reviewedAt": "2026-05-25T17:40:04.204Z",
87+
"resolved": false,
88+
"tags": ["urgent", "review"]
89+
},
90+
"annotation-2": {
91+
"jcr:primaryType": "nt:unstructured",
92+
"text": "second review note",
93+
"x": 100,
94+
"y": 200,
95+
"state": "resolved",
96+
"reviewedBy": "icreviewer",
97+
"reviewedAt": "2026-05-25T17:41:00.000Z",
98+
"resolvedBy": "admin",
99+
"resolvedAt": "2026-05-25T18:00:00.000Z"
100+
}
101+
}
102+
},
103+
"textinputWithEmptyAnnotations": {
104+
"jcr:primaryType": "nt:unstructured",
105+
"name": "nameWithEmptyAnnotations",
106+
"sling:resourceType": "forms-components-examples/components/form/textinput",
107+
"fieldType": "text-input",
108+
"fd:channel": "print",
109+
"cq:annotations": {
110+
"jcr:primaryType": "nt:unstructured",
111+
"annotation-only-jcr": {
112+
"jcr:primaryType": "nt:unstructured",
113+
"jcr:createdBy": "admin"
114+
}
115+
}
68116
}
69117
}

ui.af.apps/src/main/content/jcr_root/apps/core/fd/components/form/accordion/v1/accordion/_cq_dialog/.content.xml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,19 @@
104104
jcr:primaryType="nt:unstructured"
105105
sling:resourceType="granite/ui/components/coral/foundation/include"
106106
path="core/fd/components/form/base/v1/base/cq:dialog/content/items/tabs/items/basic/items/columns/items/column/items/readonly-typehint"/>
107+
<fieldType
108+
jcr:primaryType="nt:unstructured"
109+
sling:resourceType="granite/ui/components/coral/foundation/form/select"
110+
name="./fieldType"
111+
required="{Boolean}true"
112+
granite:hidden="true">
113+
<items jcr:primaryType="nt:unstructured">
114+
<item jcr:primaryType="nt:unstructured"
115+
text="Panel"
116+
value="panel"
117+
selected="{Boolean}true"/>
118+
</items>
119+
</fieldType>
107120
</items>
108121
</column>
109122
</items>

0 commit comments

Comments
 (0)