From 33e0be9d21262677cd1362c7a6ece6e938eaca1b Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Wed, 6 May 2026 14:52:13 +1200 Subject: [PATCH 1/4] fix(#749): handle negative step values --- packages/common/test-utils/xform-dsl/index.ts | 16 +++++ .../body/control/RangeControlDefinition.ts | 15 ++-- .../control/RangeControlDefinition.test.ts | 70 +++++++++++++++++++ 3 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts diff --git a/packages/common/test-utils/xform-dsl/index.ts b/packages/common/test-utils/xform-dsl/index.ts index e108991fe..dac27bb98 100644 --- a/packages/common/test-utils/xform-dsl/index.ts +++ b/packages/common/test-utils/xform-dsl/index.ts @@ -126,6 +126,22 @@ export const select1 = (ref: string, ...children: XFormsElement[]): XFormsElemen return t(`select1 ref="${ref}"`, ...children); }; +interface RangeAttributes { + start: string; + end: string; + step: string; +} +export const range = ( + ref: string, + attributes: RangeAttributes, + ...children: XFormsElement[] +): XFormsElement => { + return t( + `range ref="${ref}" start="${attributes.start}" end="${attributes.end}" step="${attributes.step}"`, + ...children + ); +}; + type Select1DynamicParameters = // eslint-disable-next-line @typescript-eslint/sort-type-constituents | readonly [ref: string, nodesetRef: string] diff --git a/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts b/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts index 5d611c40f..76d92ecfb 100644 --- a/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts +++ b/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts @@ -46,11 +46,18 @@ const assertNumericStringAttribute: AssertNumericStringAttribute = (localName, v } }; -const parseNumericStringAttribute = (element: Element, localName: string): NumericString => { - const value = element.getAttribute(localName); +const parseNumericStringAttribute = ( + element: Element, + localName: string, + unsign?: boolean +): NumericString => { + let value = element.getAttribute(localName); - assertNumericStringAttribute(localName, value); + if (unsign === true && value?.length && value.startsWith('-')) { + value = value.substring(1); + } + assertNumericStringAttribute(localName, value); return value; }; @@ -77,7 +84,7 @@ export class RangeControlBoundsDefinition { static from(element: Element) { const start = parseNumericStringAttribute(element, 'start'); const end = parseNumericStringAttribute(element, 'end'); - const step = parseNumericStringAttribute(element, 'step'); + const step = parseNumericStringAttribute(element, 'step', true); return new this(start, end, step); } diff --git a/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts b/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts new file mode 100644 index 000000000..08367eabc --- /dev/null +++ b/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts @@ -0,0 +1,70 @@ +import { + bind, + body, + head, + html, + mainInstance, + model, + range, + t, + title, +} from '@getodk/common/test-utils/xform-dsl/index.ts'; +import { describe, expect, it } from 'vitest'; +import { RangeControlDefinition } from '../../../../src/parse/body/control/RangeControlDefinition.ts'; +import { XFormDefinition } from '../../../../src/parse/XFormDefinition.ts'; +import { XFormDOM } from '../../../../src/parse/XFormDOM.ts'; + +describe('RangeControlDefinition', () => { + const create = (type: string, start: string, end: string, step: string) => { + const xform = html( + head( + title('Range definition'), + model( + mainInstance(t('root id="body-definition"', t('range'))), + bind('/root/range').type(type) + ) + ), + body(range('/root/range', { start, end, step })) + ); + + const xformDOM = XFormDOM.from(xform.asXml()); + const xformDefinition = new XFormDefinition(xformDOM); + const rangeElement = xformDefinition.body.element.children[0]; + + return new RangeControlDefinition(xformDefinition, xformDefinition.body, rangeElement!); + }; + + describe('bounds', () => { + describe('int', () => { + it('parses', () => { + const definition = create('int', '-2', '10', '2'); + expect(definition.bounds.start).to.equal('-2'); + expect(definition.bounds.step).to.equal('2'); + expect(definition.bounds.end).to.equal('10'); + }); + + it('takes the absolute value of step', () => { + const definition = create('int', '0', '10', '-2'); + expect(definition.bounds.start).to.equal('0'); + expect(definition.bounds.step).to.equal('2'); + expect(definition.bounds.end).to.equal('10'); + }); + }); + + describe('decimal', () => { + it('parses', () => { + const definition = create('decimal', '-2.5', '10.5', '2.5'); + expect(definition.bounds.start).to.equal('-2.5'); + expect(definition.bounds.step).to.equal('2.5'); + expect(definition.bounds.end).to.equal('10.5'); + }); + + it('takes the absolute value of step', () => { + const definition = create('decimal', '0', '10', '-2.5'); + expect(definition.bounds.start).to.equal('0'); + expect(definition.bounds.step).to.equal('2.5'); + expect(definition.bounds.end).to.equal('10'); + }); + }); + }); +}); From efe949c9d33ada3fae251e5ce77847b4e3d82c02 Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Wed, 6 May 2026 14:52:59 +1200 Subject: [PATCH 2/4] changeset --- .changeset/proud-years-chew.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/proud-years-chew.md diff --git a/.changeset/proud-years-chew.md b/.changeset/proud-years-chew.md new file mode 100644 index 000000000..b90a5fa53 --- /dev/null +++ b/.changeset/proud-years-chew.md @@ -0,0 +1,6 @@ +--- +'@getodk/xforms-engine': patch +'@getodk/common': patch +--- + +Fixed handling of ranges with negative step values From a73b84aa5bec5898ef30b67351c438a61aea584a Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Wed, 6 May 2026 15:46:34 +1200 Subject: [PATCH 3/4] improve range attributes --- packages/common/test-utils/xform-dsl/index.ts | 6 +++--- .../parse/body/control/RangeControlDefinition.test.ts | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/common/test-utils/xform-dsl/index.ts b/packages/common/test-utils/xform-dsl/index.ts index dac27bb98..e9491f277 100644 --- a/packages/common/test-utils/xform-dsl/index.ts +++ b/packages/common/test-utils/xform-dsl/index.ts @@ -127,9 +127,9 @@ export const select1 = (ref: string, ...children: XFormsElement[]): XFormsElemen }; interface RangeAttributes { - start: string; - end: string; - step: string; + start: number; + end: number; + step: number; } export const range = ( ref: string, diff --git a/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts b/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts index 08367eabc..a0317153e 100644 --- a/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts +++ b/packages/xforms-engine/test/parse/body/control/RangeControlDefinition.test.ts @@ -15,7 +15,7 @@ import { XFormDefinition } from '../../../../src/parse/XFormDefinition.ts'; import { XFormDOM } from '../../../../src/parse/XFormDOM.ts'; describe('RangeControlDefinition', () => { - const create = (type: string, start: string, end: string, step: string) => { + const create = (type: string, start: number, end: number, step: number) => { const xform = html( head( title('Range definition'), @@ -37,14 +37,14 @@ describe('RangeControlDefinition', () => { describe('bounds', () => { describe('int', () => { it('parses', () => { - const definition = create('int', '-2', '10', '2'); + const definition = create('int', -2, 10, 2); expect(definition.bounds.start).to.equal('-2'); expect(definition.bounds.step).to.equal('2'); expect(definition.bounds.end).to.equal('10'); }); it('takes the absolute value of step', () => { - const definition = create('int', '0', '10', '-2'); + const definition = create('int', 0, 10, -2); expect(definition.bounds.start).to.equal('0'); expect(definition.bounds.step).to.equal('2'); expect(definition.bounds.end).to.equal('10'); @@ -53,14 +53,14 @@ describe('RangeControlDefinition', () => { describe('decimal', () => { it('parses', () => { - const definition = create('decimal', '-2.5', '10.5', '2.5'); + const definition = create('decimal', -2.5, 10.5, 2.5); expect(definition.bounds.start).to.equal('-2.5'); expect(definition.bounds.step).to.equal('2.5'); expect(definition.bounds.end).to.equal('10.5'); }); it('takes the absolute value of step', () => { - const definition = create('decimal', '0', '10', '-2.5'); + const definition = create('decimal', 0, 10, -2.5); expect(definition.bounds.start).to.equal('0'); expect(definition.bounds.step).to.equal('2.5'); expect(definition.bounds.end).to.equal('10'); From 2183dc22e1098c03f3b2bc802941019b4ccb5018 Mon Sep 17 00:00:00 2001 From: Gareth Bowen Date: Thu, 7 May 2026 09:00:36 +1200 Subject: [PATCH 4/4] remove flag param --- .../body/control/RangeControlDefinition.ts | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts b/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts index 76d92ecfb..165096b5c 100644 --- a/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts +++ b/packages/xforms-engine/src/parse/body/control/RangeControlDefinition.ts @@ -46,18 +46,21 @@ const assertNumericStringAttribute: AssertNumericStringAttribute = (localName, v } }; -const parseNumericStringAttribute = ( - element: Element, - localName: string, - unsign?: boolean -): NumericString => { - let value = element.getAttribute(localName); +const parseNumericStringAttribute = (element: Element, localName: string): NumericString => { + const value = element.getAttribute(localName); - if (unsign === true && value?.length && value.startsWith('-')) { - value = value.substring(1); - } + assertNumericStringAttribute(localName, value); + + return value; +}; + +const abs = (value: string | null) => (value?.startsWith('-') ? value.substring(1) : value); + +const parseNumericStringAttributeAbs = (element: Element, localName: string): NumericString => { + const value = abs(element.getAttribute(localName)); assertNumericStringAttribute(localName, value); + return value; }; @@ -76,15 +79,13 @@ const parseNumericStringAttribute = ( * checking only that they appear to be numeric values. We also preserve the * attributes' names here, for consistency with the spec. * - * Downstream, we parse these to their appropriate numeric runtime types, and - * alias them to their more conventional names (i.e. "start" -> "min", "end" -> - * "max"). + * Downstream, we parse these to their appropriate numeric runtime types. */ export class RangeControlBoundsDefinition { static from(element: Element) { const start = parseNumericStringAttribute(element, 'start'); const end = parseNumericStringAttribute(element, 'end'); - const step = parseNumericStringAttribute(element, 'step', true); + const step = parseNumericStringAttributeAbs(element, 'step'); return new this(start, end, step); }