Project

General

Profile

Bug #11191

ChUI: COMBO-BOX raises error 4058 on valid SCREEN-VALUE assignment

Added by Vladimir Tsichevski 7 months ago. Updated 5 days ago.

Status:
Review
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
16409
version_resolved:
reviewer:
production:
No
env_name:
topics:

History

#2 Updated by Vladimir Tsichevski 7 months ago

Run this example in CHARACTER mode:

DEFINE VARIABLE id AS CHARACTER FORMAT "x(3)".
DEFINE FRAME f id VIEW-AS COMBO-BOX
   LIST-ITEM-PAIRS "123456789qwe", "123456789abc" 
   DROP-DOWN-LIST.
ENABLE ALL WITH FRAME f.
id:SCREEN-VALUE = "123456789abc".
WAIT-FOR WINDOW-CLOSE OF CURRENT-WINDOW.

  • In OE: code runs without error.
  • In FWD: error 4058 raised ("**Attribute SCREEN-VALUE for the %s has an invalid value of 123.").

This problem was initially reported as #11126-11.

#3 Updated by Razvan-Nicolae Chichirau 6 months ago

  • Status changed from New to WIP
Analyzed the error handling from 4GL related to setting the SCREEN-VALUE attribute. I'll use the following messages for reference:
  1. Any message that results from the fact that a value could not be converted to the combo-box format.
  2. **Attribute SCREEN-VALUE for the <widget_type> <widget_name> has an invalid value of <unformatted_value>. (4058)

For GUI combo boxes of type DROP-DOWN or SIMPLE, the value assigned to SCREEN-VALUE is not validated (i.e., whether it can be formatted or whether the formatted value exists among the items) and is simply accepted, so no errors are thrown. The following is true for DROP-DOWN-LIST on GUI and all types (SIMPLE, DROP-DOWN, DROP-DOWN-LIST) on ChUI:

  • Value can not be formatted: 1 (ERROR) + 2 (WARNING)
  • Value can be formatted, but does not exist in the items: 2 (WARNING)
  • Value is empty string
    • no items: no error
    • has items: no error
  • Value is unknown value:
    • no items: no error
    • has items: no error

#4 Updated by Razvan-Nicolae Chichirau 6 months ago

  • Assignee set to Razvan-Nicolae Chichirau

#5 Updated by Razvan-Nicolae Chichirau 5 months ago

Did some extensive testing for the following control set entities: radio-set, combo-box and selection-list. Consider the error messages from #11191-3.

COMBO-BOX

The following data-types were tested: character, logical, integer, decimal, date, datetime, datetime-tz, recid
  • GUI
    • The SIMPLE and DROP-DOWN combos does not throw any errors and accept the input as is.
    • For DROP-DOWN-LIST:
      1. Widget is not realized
        • Can not format the screen-value: 1 + 2
        • Can format, but does not exist among the current items: no errors, silent exit
      2. Widget is realized
        • Can not format the screen-value: 1 + 2
        • Can format, but does not exist among the current items: 2
  • CHUI
    • The following is true for all combo-box types:
      1. Widget is not realized
        • Can not format the screen-value: 1 + 2
        • Can format, but does not exist among the current items: no errors, silent exit
      2. Widget is realized
        • Can not format the screen-value: 1 + 2
        • Can format, but does not exist among the current items: 2

RADIO-SET

The following data-types were tested: character, logical, integer, decimal, date, datetime, datetime-tz, recid
  • GUI + CHUI
    • Widget is not realized or it is realized: 2

SELECTION-LIST

Only character data-type was tested as it was the only one permitted for this widget.
  • GUI + CHUI:
    • Widget is not realized or it is realized: 2

The browse column widget has another behavior compared with the widgets listed above. This can be treated in another task and the functionality of combo-box, radio-set and selection-list can be grouped and treated in this issue.

#6 Updated by Razvan-Nicolae Chichirau 4 months ago

  • % Done changed from 0 to 100
  • Status changed from WIP to Review

Hynek: Please review 11191a/rev. 16549.

#7 Updated by Hynek Cihlar 3 months ago

  • Status changed from Review to WIP
  • % Done changed from 100 to 90

Code review 11191a revisions 16548..16549

  • [CRITICAL] style RadioSetWidget: Copyright year in the file header is still 2005-2025, but the file is modified in 2026. Update to 2005-2026, Golden Code Development Corporation.
  • [CRITICAL] functional ControlSetEntity.invalidScreenValueWarning: Method dereferences unparsedValue via unparsedValue.isUnknown() with no null check. Concrete trigger: the 3-arg GenericWidget.setScreenValue(ScreenBuffer, Object, boolean) forwards unparsedValue=null to the 4-arg overload; this is exercised on the standard frame realization / DISPLAY path via GenericFrame.copyToScreenBuffer (around line 11301) on a RADIO-SET, SELECTION-LIST or already-realized COMBO-BOX whose value fails isValidScreenValue. The chain reaches validateScreenValue(value, null) which calls invalidScreenValueWarning(null, ...); for RADIO-SET / SELECTION-LIST the realizeCheck=false guard does not bail out, and the method NPEs on unparsedValue.isUnknown(). The old inline implementation referenced only the parsed value (never null), so this is a regression. Add a null guard or fall back to the parsed value when unparsedValue is null.
  • [CRITICAL] functional RadioSetWidget.validateScreenValue: Calls invalidScreenValueWarning(unparsedValue, false) when validation fails. Same trigger as above — unparsedValue can be null on the 3-arg setScreenValue path (used during frame realization). With realizeCheck=false and a non-null frame, invalidScreenValueWarning(null, false) immediately NPEs on unparsedValue.isUnknown().
  • [MAJOR] functional ControlSetEntity.invalidScreenValueWarning: Replaces ErrorManager.recordOrShowWarning(...) with ErrorManager.displayWarning(...). The two are not equivalent: (a) recordOrShowWarning short-circuits on isSuppressWarnings() and on per-code suppressWarningsSet.contains(num), while displayWarning never consults the per-code suppression set and adds the condition to _MSG even under SESSION:SUPPRESS-WARNINGS; (b) recordOrShowWarning delegates to recordOrShowError which throws DeferredLegacyErrorException when hasLegacyError()/mustManageLegacyError() is true, allowing an enclosing 4GL CATCH Progress.Lang.SysError to capture the condition — displayWarning never throws. Reachable from straightforward 4GL: ASSIGN cb:SCREEN-VALUE = "<invalid>" inside SESSION:SUPPRESS-WARNINGS, with SUPPRESS-WARNINGS-LIST naming 4056/4058, or inside a CATCH block. Verify against OE behavior; if displayWarning is the intended API, the previously suppressed/catchable cases need explicit handling.
  • [MINOR] style RadioSetWidget.setScreenValueInt: Continuation lines of the multi-line if condition are indented one column short of the opening condition character, breaking project convention for multi-line condition alignment.
  • [MINOR] style GenericFrame.setScreenValue: Newly added Javadoc \@param widget description reads "The widget for which the value is to be retrieved". This is a setter; description must say "set".
  • [MINOR] style ControlSetEntity.invalidScreenValueWarning: Both \@param tags use 3 spaces after the tag keyword. All other Javadoc \@param tags in this file and in the same diff use 4 spaces — adjust for consistency.
  • [MINOR] style ControlSetEntity.shouldParseScreenValue / ComboBoxWidget.shouldParseScreenValue: Both newly added Javadoc blocks use \@return with 2 spaces. Existing \@return tags in these files use 3 spaces — adjust for consistency.

Instead of getSubType().getValue().equals("DROP-DOWN-LIST") compare the enum value returned by ComboBoxConfig.getMode.

In setScreenValueInt catching ErrorConditionException is used for the happy path. The catch block here is part of the expected flow, not an exceptional condition. Let's turn this into a predicate (e.g. tryParseScreenValue returning a boolean) and reserve the exception for genuinely unexpected failures (or the error condition in this case).

Is the 'UNKNOWN' in invalidScreenValueWarning also displayed in OpenEdge? OpenEdge AFAIK typically represents unknown values as '?' in the messages.

#8 Updated by Razvan-Nicolae Chichirau 3 months ago

  • % Done changed from 90 to 100
  • Status changed from WIP to Review

Hynek Cihlar wrote:

Code review 11191a revisions 16548..16549

  • [CRITICAL] style RadioSetWidget: Copyright year in the file header is still 2005-2025, but the file is modified in 2026. Update to 2005-2026, Golden Code Development Corporation.
  • [CRITICAL] functional ControlSetEntity.invalidScreenValueWarning: Method dereferences unparsedValue via unparsedValue.isUnknown() with no null check. Concrete trigger: the 3-arg GenericWidget.setScreenValue(ScreenBuffer, Object, boolean) forwards unparsedValue=null to the 4-arg overload; this is exercised on the standard frame realization / DISPLAY path via GenericFrame.copyToScreenBuffer (around line 11301) on a RADIO-SET, SELECTION-LIST or already-realized COMBO-BOX whose value fails isValidScreenValue. The chain reaches validateScreenValue(value, null) which calls invalidScreenValueWarning(null, ...); for RADIO-SET / SELECTION-LIST the realizeCheck=false guard does not bail out, and the method NPEs on unparsedValue.isUnknown(). The old inline implementation referenced only the parsed value (never null), so this is a regression. Add a null guard or fall back to the parsed value when unparsedValue is null.
  • [CRITICAL] functional RadioSetWidget.validateScreenValue: Calls invalidScreenValueWarning(unparsedValue, false) when validation fails. Same trigger as above — unparsedValue can be null on the 3-arg setScreenValue path (used during frame realization). With realizeCheck=false and a non-null frame, invalidScreenValueWarning(null, false) immediately NPEs on unparsedValue.isUnknown().
  • [MINOR] style RadioSetWidget.setScreenValueInt: Continuation lines of the multi-line if condition are indented one column short of the opening condition character, breaking project convention for multi-line condition alignment.
  • [MINOR] style GenericFrame.setScreenValue: Newly added Javadoc \@param widget description reads "The widget for which the value is to be retrieved". This is a setter; description must say "set".
  • [MINOR] style ControlSetEntity.invalidScreenValueWarning: Both \@param tags use 3 spaces after the tag keyword. All other Javadoc \@param tags in this file and in the same diff use 4 spaces — adjust for consistency.
  • [MINOR] style ControlSetEntity.shouldParseScreenValue / ComboBoxWidget.shouldParseScreenValue: Both newly added Javadoc blocks use \@return with 2 spaces. Existing \@return tags in these files use 3 spaces — adjust for consistency.

Instead of getSubType().getValue().equals("DROP-DOWN-LIST") compare the enum value returned by ComboBoxConfig.getMode.

In setScreenValueInt catching ErrorConditionException is used for the happy path. The catch block here is part of the expected flow, not an exceptional condition. Let's turn this into a predicate (e.g. tryParseScreenValue returning a boolean) and reserve the exception for genuinely unexpected failures (or the error condition in this case).

Fixed.

  • [MAJOR] functional ControlSetEntity.invalidScreenValueWarning: Replaces ErrorManager.recordOrShowWarning(...) with ErrorManager.displayWarning(...). The two are not equivalent: (a) recordOrShowWarning short-circuits on isSuppressWarnings() and on per-code suppressWarningsSet.contains(num), while displayWarning never consults the per-code suppression set and adds the condition to _MSG even under SESSION:SUPPRESS-WARNINGS; (b) recordOrShowWarning delegates to recordOrShowError which throws DeferredLegacyErrorException when hasLegacyError()/mustManageLegacyError() is true, allowing an enclosing 4GL CATCH Progress.Lang.SysError to capture the condition — displayWarning never throws. Reachable from straightforward 4GL: ASSIGN cb:SCREEN-VALUE = "<invalid>" inside SESSION:SUPPRESS-WARNINGS, with SUPPRESS-WARNINGS-LIST naming 4056/4058, or inside a CATCH block. Verify against OE behavior; if displayWarning is the intended API, the previously suppressed/catchable cases need explicit handling.
Raising a warning for the invalid screen-value in 4GL will:
  • Make the warning be unconditionally added to _MSG, whether SESSION:SUPPRESS-WARNINGS or set or not
  • Not display the warning in case of SESSION:SUPPRESS-WARNINGS
  • Not throw an actual 4GL legacy error which can be caught by a CATCH block.

As such, displayWarning is the correct API to use here.

Is the 'UNKNOWN' in invalidScreenValueWarning also displayed in OpenEdge? OpenEdge AFAIK typically represents unknown values as '?' in the messages.

Check this testcase:

def var rs as char view-as radio-set radio-buttons "a", "b".
def frame f rs.

rs:screen-value = ?.

Please review 11191a/rev. 16550.

#9 Updated by Hynek Cihlar 3 months ago

  • Status changed from Review to Internal Test

Code review 11191a revisions 16548..16550

The changes look good. Please go ahead with regression testing. Also run ChUI regression tests.

#10 Updated by Razvan-Nicolae Chichirau about 2 months ago

  • Status changed from Internal Test to Review

Small regression detected on the testcases project:

define variable rs as logical initial ? format 'one/two' view-as radio-set radio-buttons "first", true, "second", true.
def frame f rs.

rs:screen-value = "".
display rs with frame f.
wait-for window-close of current-window. 

Trunk and 4GL do not raise a warning, meanwhile 11191a does, GUI + ChUI.

Hynek: Please review 11191a/rev. 16624.

#11 Updated by Hynek Cihlar about 1 month ago

  • Status changed from Review to WIP
  • % Done changed from 100 to 90

Code review 11191a revisions 16621..16624

Intsead of this instanceof BrowseColumnWidget override recoverFromParsingError.

Please check the following functional points.

  • [MAJOR] functional RadioSetWidget.setScreenValueInt/isValidScreenValue: The rev-16624 savedValue recovery that suppresses the false-positive 4058 warning is captured only inside the GUI branch (!LogicalTerminal.isChui()), so the ChUI arm of the reproducer regresses. In ChUI, setScreenValueInt leaves "" untouched and savedValue stays null; parseScreenValue(logical, "", "one/two") yields an unknown logical (empty value, format not delimited by /), which ControlSetEntity.setScreenValue re-derives as an unknown character; isValidScreenValue then has no savedValue to restore the original "", returns false on !value.isUnknown(), and validateScreenValue fires a spurious 4058. GUI works only because it captures savedValue = "" and swaps it back. Trigger: the redmine section-3 reproducer (rs as logical initial ? format 'one/two' view-as radio-set ...; rs:screen-value = "") run in CHARACTER mode — redmine states the false positive appeared in both GUI and ChUI on 11191a. Capture savedValue (or otherwise preserve the original value for validation) on the ChUI path as well.
  • [MINOR] functional ControlSetEntity.invalidScreenValueWarning: The switch from ErrorManager.recordOrShowWarning to ErrorManager.displayWarning(id, msg, true) drops honoring of SESSION:SUPPRESS-WARNINGS-LIST. recordOrShowWarning suppresses when either the global SUPPRESS-WARNINGS boolean or the per-number set suppressWarningsSet matches (ErrorManager ~line 1452-1454); displayWarning checks only the global boolean (~line 3441) and never consults suppressWarningsSet. Trigger: a program sets SESSION:SUPPRESS-WARNINGS-LIST = "4058" (or "4056") then assigns an invalid SCREEN-VALUE to a realized control-set widget — previously suppressed entirely, now displayed and added to _MSG. Redmine section 2 mandates only the global SUPPRESS-WARNINGS for this case and requires the warning to unconditionally reach _MSG, so this may be acceptable; verify the per-number-list behavior against OE.
  • [MINOR] style ComboBoxWidget (header history entry #057): Typo in the new history description — "Overriden 'shouldParseScreenValue()'." should be "Overridden".

#12 Updated by Razvan-Nicolae Chichirau about 1 month ago

  • % Done changed from 90 to 100
  • Status changed from WIP to Review

Hynek Cihlar wrote:

Intsead of this instanceof BrowseColumnWidget override recoverFromParsingError.

  • [MAJOR] functional RadioSetWidget.setScreenValueInt/isValidScreenValue: The rev-16624 savedValue recovery that suppresses the false-positive 4058 warning is captured only inside the GUI branch (!LogicalTerminal.isChui()), so the ChUI arm of the reproducer regresses. In ChUI, setScreenValueInt leaves "" untouched and savedValue stays null; parseScreenValue(logical, "", "one/two") yields an unknown logical (empty value, format not delimited by /), which ControlSetEntity.setScreenValue re-derives as an unknown character; isValidScreenValue then has no savedValue to restore the original "", returns false on !value.isUnknown(), and validateScreenValue fires a spurious 4058. GUI works only because it captures savedValue = "" and swaps it back. Trigger: the redmine section-3 reproducer (rs as logical initial ? format 'one/two' view-as radio-set ...; rs:screen-value = "") run in CHARACTER mode — redmine states the false positive appeared in both GUI and ChUI on 11191a. Capture savedValue (or otherwise preserve the original value for validation) on the ChUI path as well.
  • [MINOR] style ComboBoxWidget (header history entry #057): Typo in the new history description — "Overriden 'shouldParseScreenValue()'." should be "Overridden".

Solved.

  • [MINOR] functional ControlSetEntity.invalidScreenValueWarning: The switch from ErrorManager.recordOrShowWarning to ErrorManager.displayWarning(id, msg, true) drops honoring of SESSION:SUPPRESS-WARNINGS-LIST. recordOrShowWarning suppresses when either the global SUPPRESS-WARNINGS boolean or the per-number set suppressWarningsSet matches (ErrorManager ~line 1452-1454); displayWarning checks only the global boolean (~line 3441) and never consults suppressWarningsSet. Trigger: a program sets SESSION:SUPPRESS-WARNINGS-LIST = "4058" (or "4056") then assigns an invalid SCREEN-VALUE to a realized control-set widget — previously suppressed entirely, now displayed and added to _MSG. Redmine section 2 mandates only the global SUPPRESS-WARNINGS for this case and requires the warning to unconditionally reach _MSG, so this may be acceptable; verify the per-number-list behavior against OE.

4GL suppresses the warning message if either SESSION:SUPPRESS-WARNINGS is used or the message number is in the SESSION:SUPPRESS-WARNINGS-LIST value. Additionally, it adds it to _MSG() only if silent error is disabled and the process is not running in headless mode. As such, the correct method is recordOrShowWarning(), but with a slight modification. If the warning is suppressed and silent mode is disabled + not running in headless mode, append the message number to _MSG().

Note that I've verified all usages of this method, and the behavior is the same in every case. Please check rev. 16625.

#13 Updated by Hynek Cihlar 8 days ago

  • Status changed from Review to WIP
  • % Done changed from 100 to 90

Code review 11191a revisions 16621..16625

The code changes look good (just a couple of minor styling issues, please see below), but there are some functional issues. The CRITICAL one is confirmed. I didn't check the MAJOR ones, please do that.

  • [CRITICAL] functional GenericFrame.setScreenValue: On the new shouldParseScreenValue() false branch, val = value aliases the caller's live character instead of the fresh object the old path always returned (parseValue line 3944 does result = new character(value)). val, unparsedValue and the application's own variable then become one object: ControlSetEntity.setScreenValue line 3011 does val = (character) value with no copy, and ComboBoxWidget.validateScreenValue mutates it in place — value.assign(itemValue) (line 2221), value.assign(TextOps.rightTrim(value)) (2231), value.assign("") (2237). No earlier layer copies: GenericWidget.setScreenValue(character) copies nothing, setScreenValueNoErrorHandling duplicates only on the frame null path, and the conversion rule (rules/convert/methods_attributes.rules:5011-5017) emits a bare setScreenValue(<expr>) passing the live BDT (see Weblist.java:597, Webradio.java:506). Trigger (GUI, mode SIMPLE or DROP-DOWN): def var c as char init " ". def var cb as char view-as combo-box simple list-items "a","b". def frame f cb. display cb with frame f. cb:screen-value = c. message length(c). — FWD now prints 0 because line 2237 rewrote the application's c; it printed 3 before. Variant: c = "a " is rewritten to "a" by the rightTrim/assign(itemValue) calls. Second effect: ScreenBuffer.putWidgetValue (line 474, swr.value = value) and putScreenValue (line 622) store the reference verbatim, so a later c = "zzz" (an in-place c.assign) silently changes what cb:SCREEN-VALUE returns, diverging from what was already painted on the client. Fix: val = (character) value.duplicate();.
  • [MAJOR] functional ControlSetEntity.validateScreenValue: The issue's own CHARACTER-mode reproducer still raises 4058 — only the printed value changes ("123" becomes "123456789abc"). The ChUI unparsed-value clause (LogicalTerminal.isChui() && unparsedValue != null && !isValidScreenValue(unparsedValue)) is a pure context line in the diff. For DEF VAR id AS CHAR FORMAT "x(3)" + LIST-ITEM-PAIRS "123456789qwe","123456789abc" DROP-DOWN-LIST: parseScreenValue(character, "123456789abc", "x(3)") does not throw (StringFormat.CharBuf.parseScreenValue line 1764 appends the first 3 chars; x accepts any char so checkFormat passes), giving val = "123" with unparsedValue the untouched 12-char value. Item values are truncated by the same format — ComboBoxWidget.setItems line 1695 does item.setValue(TextOps.rightTrim(new character(item.getValue().toString(fmt)))) with fmt = resolveFormat() = "x(3)" — so the item value is also "123". ControlSetEntity.isValidScreenValue line 3099 uses exact CompareOps._isEqual only (no "begins of" leniency; the TextOps._begins match in getItemValue runs only after validation passes and tests the wrong direction), so isValidScreenValue("123456789abc") is false, the ChUI disjunct fires, and invalidScreenValueWarning(..., realizeCheck=true) emits 4058 on the widget realized by ENABLE ALL. The outcome is robust to the truncation question: were the items not truncated, the first disjunct would fire instead. Validating the raw unparsed value against the item list is the residual defect — a value that formats to an existing item must be accepted silently.
  • [MAJOR] functional GenericFrame.setScreenValue: The new recoverFromParsingError hook is wired only to catch (DisplayFormatCheckException e) (lines 3786-3804), but parseValue (3929-4014) can raise that exception only in the type.equals(character.class) branch via pres.checkFormat() (StringFormat.java:1123). For the numeric and date datatypes the "cannot format" case raises ErrorConditionException instead, which bypasses the hook entirely: integer/int64/decimal via NumberType.parseDecimal to errorInvalidChar (NumberType.java:3495) to recordOrThrowError(76, ...), and date/datetime/datetime-tz via new date(character) to recordOrStoreError85 (date.java:5137) to recordOrThrowError(85, ...). ErrorConditionException is a RuntimeException caught nowhere between parseScreenValue and GenericWidget.setScreenValueInt (lines 6731-6744), which emits 4078; invalidScreenValueWarning is never reached and ControlSetEntity.validateScreenValue is never entered. Trigger (ChUI, realized): def var i as int view-as combo-box list-items "1","2","3". def frame f i. enable all with frame f. i:screen-value = "abc". — FWD emits 76 + 4078 where the matrix requires the format error plus 4058; same for a date combo (85 + 4078). So the central fix works for character and logical only (logical is fine: new logical(character, format) records 87 and returns unknown without throwing, so the flow reaches validateScreenValue and 4058 fires). Note the FIXME: we need to find out which error number to use here comment survives untouched exactly where these types land. Related gap: recid is not handled by parseValue at all (recid extends int64 but the branch uses exact type.equals(int64.class)), so a recid control set skips parsing and format checking entirely — no error of any kind. Both integer/decimal/date/datetime/datetime-tz and recid are listed in the issue as "covered by testing".
  • [MAJOR] functional ControlSetEntity.validateScreenValue: Replacing the old frame != null && getAttr(ControlSetConfig::getWasRealized, config, true) guard with invalidScreenValueWarning(..., this instanceof ComboBoxWidget) drops the realization gate for SELECTION-LIST on the UI-statement path, not just the SCREEN-VALUE attribute path. GenericFrame.display() (line 6011) calls copyToScreenBuffer before viewWorker (6013), and copyToScreenBuffer line 11311 calls widget.setScreenValue(frameBuf, value, true) with inUIStmt = true; ControlSetEntity.setScreenValue line 3002 (if (inUIStmt || !internalScreenValueUsage)) then runs validateScreenValue while wasRealized is still false. Trigger: DEFINE VARIABLE sl AS CHARACTER INITIAL "zzz" VIEW-AS SELECTION-LIST LIST-ITEMS "a","b". DEFINE FRAME f sl. DISPLAY sl WITH FRAME f. now emits 4058 on the first DISPLAY where the old realization gate kept it silent; the same reaches updateWorker (9846, 9928 — UPDATE/SET), displayWorker (12012), displayAndDownWorker (13594) and startEditingMode (8066). Nothing blocks the path — SelectionListWidget.hasFormat() returning false affects only format handling, and SelectionListWidget.validateScreenValue (960) delegates unconditionally to super. The matrix rows describe SCREEN-VALUE assignment; a plain DISPLAY would emit a message naming an attribute the program never touched. Decisive in-diff evidence that the two halves are inconsistent: the author added if (!internalScreenValueUsage) to RadioSetWidget.validateScreenValue (1435-1439) with the comment "The RADIO-SET widget throws invalid screen-value warnings only for external calls" — RADIO-SET and SELECTION-LIST share a matrix row, yet only one got the gate, so identical 4GL now behaves differently. Suggested fix: gate the ControlSetEntity warning on !internalScreenValueUsage too. Aggravating factor (mechanism pre-existing): recordOrShowWarning passes isError=false and recordOrShowError re-derives isError = hasLegacyError() || mustManageLegacyError(), so inside a CATCH or BLOCK-LEVEL UNDO,THROW block this becomes a DeferredLegacyErrorException — an ERROR condition raised by a plain DISPLAY.
  • [MAJOR] functional RadioSetWidget.setScreenValueInt: Removing the !LogicalTerminal.isChui() guard makes the empty-string-to-unknown coercion run on ChUI, changing the stored and rendered value, not just validation. savedValue is consulted only inside isValidScreenValue; the item-matching loop in validateScreenValue compares the parameter (the coerced unknown), and both frameBuf.putWidgetValue and refreshFrameWidget/putScreenValue receive the unknown. CompareOps._isEqual(unknown, "") is false, and client-side RadioSet.setValue only ever selectButton@s on a match, so an unknown is a no-op on the client — no button selected, @config.current left as-is. Trigger (ChUI, empty-valued item present, widget realized before the assignment so getInitialValue() does not mask it): def var c as char view-as radio-set radio-buttons "Nothing","","A","a". def frame f c. display c with frame f. c:screen-value = "". — previously "" reached the loop, matched the empty-valued item and was stored, selecting that button; now nothing matches. The guard being removed was added deliberately by the same author at rev. 15471 (#8836) together with the "for GUI clients" Javadoc, so ChUI non-coercion was established behavior, and nothing in the issue claims OE ChUI was re-tested. ComboBoxWidget.setScreenValueInt still keeps both !isChui and !pairs guards on the same coercion, and RadioSetConfig sets pairs = true unconditionally — by that analogy the quirk should apply less here, not more. The coercion is also not required by the rev-16624 fix: capturing savedValue unconditionally while keeping the isChui guard on the coercion is sufficient, because parseScreenValue(logical, "", 'one/two') already yields an unknown on ChUI, so isValidScreenValue still gets its savedValue and no 4058 is emitted. Separately, hoisting return true out of the item loop means rs:screen-value = "" with no empty-valued item is now accepted (storing unknown) where the old fall-through return false made GenericFrame skip putScreenValue/refreshFrameWidget and leave the selection untouched.
  • [MAJOR] functional GenericFrame.setScreenValue: The same non-parsing branch loses the unknown-value typing, regressing an intentional OE behavior. ComboBoxWidget.resolveUnknownScreenValue (lines 2017-2020) branches on the buffer value's class — bdt.getClass().isAssignableFrom(date.class) ? new character("") : new character() — and GenericFrame line 3662 documents the rule ("date/datetime/datetime-tz all return empty string for unknown", implemented per history entry 048 FER 20250312). Before the change parseValue (3930-3941) short-circuited unknown input to BaseDataTypeFactory.instantiate(type), i.e. an unknown date, so the test passed and the read-back was ""; now the buffer holds an unknown character, the test fails, and the read-back is ?. Trigger (GUI): DEFINE VARIABLE d AS DATE VIEW-AS COMBO-BOX SUBTYPE SIMPLE LIST-ITEMS "01/01/2020". then d:SCREEN-VALUE = ? then read d:SCREEN-VALUE. Scope: confined to plain DATE — for datetime/datetimetz the pre-existing isAssignableFrom test is written backwards and was already false, and for the numeric types both before and after yield unknown.
  • [MAJOR] functional GenericFrame.convertScreenValue: Letting an unparseable value into the screen buffer (correct per the matrix, "accepted as-is") makes a downstream read silently swallow a raised 4GL error and substitute the type default. ComboBoxWidget.inputValue to GenericWidget.inputValue to frame.getter(id, integer.class, true, false) to convertScreenValue: the (character, String) constructor lookup fails into the blanket catch (Exception exc) at line 8841, then integer.getConstructor(character.class) to newInstance to int64.setValue(String) to NumberType.parseDouble("abc") to errorInvalidChar to recordOrThrowError(76, "Invalid character in numeric input a") — and that ErrorConditionException (wrapped in InvocationTargetException) is discarded by the second blanket catch (Exception exc) at line 8861, leaving value null so defaults true substitutes BaseDataType.generateDefault(integer.class). Trigger (GUI): def var i as int view-as combo-box simple list-items "1","2". def frame f i. i:screen-value = "abc". message i:input-value. prints 0 and error 76 never surfaces — not even in ERROR-STATUS. The blanket catch is pre-existing, but this diff is what makes the path reachable: before the change an unparseable value could never enter the buffer for a GUI SIMPLE/DROP-DOWN combo. Caveat: inputValue deliberately maps an unknown result to generateDefault (history entries 038 SAT 20231025/26), so the returned 0 may be OE-correct; the destroyed error condition is a defect regardless. Worth confirming on OE whether 76 (or 5321) surfaces for INPUT-VALUE of an unparseable integer combo screen value.
  • [MINOR] style ControlSetEntity.invalidScreenValueWarning: The new protected instance method (line 3291) is placed after the private instance methods controlSetItem/sortItems and the private static method bubbleSort, violating the required member ordering (protected instance methods must precede private static and instance methods). Relocate it up among the other protected instance methods, e.g. near recoverFromParsingError/isValidScreenValue.
  • [MINOR] style RadioSetWidget: The new field comment for savedValue (line 145) uses a plain block comment /* ... */ instead of the javadoc /** ... */ style used for field documentation elsewhere in the package (e.g. every field in GenericWidget). Convert it to /** ... */.

#14 Updated by Razvan-Nicolae Chichirau 5 days ago

  • % Done changed from 90 to 100
  • Status changed from WIP to Review

Hynek Cihlar wrote:

  • [CRITICAL] functional GenericFrame.setScreenValue: On the new shouldParseScreenValue() false branch, val = value aliases the caller's live character instead of the fresh object the old path always returned (parseValue line 3944 does result = new character(value)). val, unparsedValue and the application's own variable then become one object: ControlSetEntity.setScreenValue line 3011 does val = (character) value with no copy, and ComboBoxWidget.validateScreenValue mutates it in place — value.assign(itemValue) (line 2221), value.assign(TextOps.rightTrim(value)) (2231), value.assign("") (2237). No earlier layer copies: GenericWidget.setScreenValue(character) copies nothing, setScreenValueNoErrorHandling duplicates only on the frame null path, and the conversion rule (rules/convert/methods_attributes.rules:5011-5017) emits a bare setScreenValue(<expr>) passing the live BDT (see Weblist.java:597, Webradio.java:506). Trigger (GUI, mode SIMPLE or DROP-DOWN): def var c as char init " ". def var cb as char view-as combo-box simple list-items "a","b". def frame f cb. display cb with frame f. cb:screen-value = c. message length(c). — FWD now prints 0 because line 2237 rewrote the application's c; it printed 3 before. Variant: c = "a " is rewritten to "a" by the rightTrim/assign(itemValue) calls. Second effect: ScreenBuffer.putWidgetValue (line 474, swr.value = value) and putScreenValue (line 622) store the reference verbatim, so a later c = "zzz" (an in-place c.assign) silently changes what cb:SCREEN-VALUE returns, diverging from what was already painted on the client. Fix: val = (character) value.duplicate();.

Solved.

  • [MAJOR] functional ControlSetEntity.validateScreenValue: The issue's own CHARACTER-mode reproducer still raises 4058 — only the printed value changes ("123" becomes "123456789abc"). The ChUI unparsed-value clause (LogicalTerminal.isChui() && unparsedValue != null && !isValidScreenValue(unparsedValue)) is a pure context line in the diff. For DEF VAR id AS CHAR FORMAT "x(3)" + LIST-ITEM-PAIRS "123456789qwe","123456789abc" DROP-DOWN-LIST: parseScreenValue(character, "123456789abc", "x(3)") does not throw (StringFormat.CharBuf.parseScreenValue line 1764 appends the first 3 chars; x accepts any char so checkFormat passes), giving val = "123" with unparsedValue the untouched 12-char value. Item values are truncated by the same format — ComboBoxWidget.setItems line 1695 does item.setValue(TextOps.rightTrim(new character(item.getValue().toString(fmt)))) with fmt = resolveFormat() = "x(3)" — so the item value is also "123". ControlSetEntity.isValidScreenValue line 3099 uses exact CompareOps._isEqual only (no "begins of" leniency; the TextOps._begins match in getItemValue runs only after validation passes and tests the wrong direction), so isValidScreenValue("123456789abc") is false, the ChUI disjunct fires, and invalidScreenValueWarning(..., realizeCheck=true) emits 4058 on the widget realized by ENABLE ALL. The outcome is robust to the truncation question: were the items not truncated, the first disjunct would fire instead. Validating the raw unparsed value against the item list is the residual defect — a value that formats to an existing item must be accepted silently.

Testcase as described in the bullet point:

DEF VAR id AS CHAR view-as combo-box LIST-ITEM-PAIRS "123456789qwe","123456789abc" DROP-DOWN-LIST FORMAT "x(3)".
def frame f id.

enable all with frame f.
wait-for window-close of current-window.

4GL and 11191a does not report anything in ChUI.

  • [MAJOR] functional GenericFrame.setScreenValue: The new recoverFromParsingError hook is wired only to catch (DisplayFormatCheckException e) (lines 3786-3804), but parseValue (3929-4014) can raise that exception only in the type.equals(character.class) branch via pres.checkFormat() (StringFormat.java:1123). For the numeric and date datatypes the "cannot format" case raises ErrorConditionException instead, which bypasses the hook entirely: integer/int64/decimal via NumberType.parseDecimal to errorInvalidChar (NumberType.java:3495) to recordOrThrowError(76, ...), and date/datetime/datetime-tz via new date(character) to recordOrStoreError85 (date.java:5137) to recordOrThrowError(85, ...). ErrorConditionException is a RuntimeException caught nowhere between parseScreenValue and GenericWidget.setScreenValueInt (lines 6731-6744), which emits 4078; invalidScreenValueWarning is never reached and ControlSetEntity.validateScreenValue is never entered. Trigger (ChUI, realized): def var i as int view-as combo-box list-items "1","2","3". def frame f i. enable all with frame f. i:screen-value = "abc". — FWD emits 76 + 4078 where the matrix requires the format error plus 4058; same for a date combo (85 + 4078). So the central fix works for character and logical only (logical is fine: new logical(character, format) records 87 and returns unknown without throwing, so the flow reaches validateScreenValue and 4058 fires). Note the FIXME: we need to find out which error number to use here comment survives untouched exactly where these types land. Related gap: recid is not handled by parseValue at all (recid extends int64 but the branch uses exact type.equals(int64.class)), so a recid control set skips parsing and format checking entirely — no error of any kind. Both integer/decimal/date/datetime/datetime-tz and recid are listed in the issue as "covered by testing".

Valid. Extended the catch.

  • [MAJOR] functional ControlSetEntity.validateScreenValue: Replacing the old frame != null && getAttr(ControlSetConfig::getWasRealized, config, true) guard with invalidScreenValueWarning(..., this instanceof ComboBoxWidget) drops the realization gate for SELECTION-LIST on the UI-statement path, not just the SCREEN-VALUE attribute path. GenericFrame.display() (line 6011) calls copyToScreenBuffer before viewWorker (6013), and copyToScreenBuffer line 11311 calls widget.setScreenValue(frameBuf, value, true) with inUIStmt = true; ControlSetEntity.setScreenValue line 3002 (if (inUIStmt || !internalScreenValueUsage)) then runs validateScreenValue while wasRealized is still false. Trigger: DEFINE VARIABLE sl AS CHARACTER INITIAL "zzz" VIEW-AS SELECTION-LIST LIST-ITEMS "a","b". DEFINE FRAME f sl. DISPLAY sl WITH FRAME f. now emits 4058 on the first DISPLAY where the old realization gate kept it silent; the same reaches updateWorker (9846, 9928 — UPDATE/SET), displayWorker (12012), displayAndDownWorker (13594) and startEditingMode (8066). Nothing blocks the path — SelectionListWidget.hasFormat() returning false affects only format handling, and SelectionListWidget.validateScreenValue (960) delegates unconditionally to super. The matrix rows describe SCREEN-VALUE assignment; a plain DISPLAY would emit a message naming an attribute the program never touched. Decisive in-diff evidence that the two halves are inconsistent: the author added if (!internalScreenValueUsage) to RadioSetWidget.validateScreenValue (1435-1439) with the comment "The RADIO-SET widget throws invalid screen-value warnings only for external calls" — RADIO-SET and SELECTION-LIST share a matrix row, yet only one got the gate, so identical 4GL now behaves differently. Suggested fix: gate the ControlSetEntity warning on !internalScreenValueUsage too. Aggravating factor (mechanism pre-existing): recordOrShowWarning passes isError=false and recordOrShowError re-derives isError = hasLegacyError() || mustManageLegacyError(), so inside a CATCH or BLOCK-LEVEL UNDO,THROW block this becomes a DeferredLegacyErrorException — an ERROR condition raised by a plain DISPLAY.

Both 4GL and 11191a raises 4058 in this testcase, so if the trunk was silent on the DISPLAY statement, it was a bug which is now solved.

  • [MAJOR] functional RadioSetWidget.setScreenValueInt: Removing the !LogicalTerminal.isChui() guard makes the empty-string-to-unknown coercion run on ChUI, changing the stored and rendered value, not just validation. savedValue is consulted only inside isValidScreenValue; the item-matching loop in validateScreenValue compares the parameter (the coerced unknown), and both frameBuf.putWidgetValue and refreshFrameWidget/putScreenValue receive the unknown. CompareOps._isEqual(unknown, "") is false, and client-side RadioSet.setValue only ever selectButton@s on a match, so an unknown is a no-op on the client — no button selected, @config.current left as-is. Trigger (ChUI, empty-valued item present, widget realized before the assignment so getInitialValue() does not mask it): def var c as char view-as radio-set radio-buttons "Nothing","","A","a". def frame f c. display c with frame f. c:screen-value = "". — previously "" reached the loop, matched the empty-valued item and was stored, selecting that button; now nothing matches. The guard being removed was added deliberately by the same author at rev. 15471 (#8836) together with the "for GUI clients" Javadoc, so ChUI non-coercion was established behavior, and nothing in the issue claims OE ChUI was re-tested. ComboBoxWidget.setScreenValueInt still keeps both !isChui and !pairs guards on the same coercion, and RadioSetConfig sets pairs = true unconditionally — by that analogy the quirk should apply less here, not more. The coercion is also not required by the rev-16624 fix: capturing savedValue unconditionally while keeping the isChui guard on the coercion is sufficient, because parseScreenValue(logical, "", 'one/two') already yields an unknown on ChUI, so isValidScreenValue still gets its savedValue and no 4058 is emitted. Separately, hoisting return true out of the item loop means rs:screen-value = "" with no empty-valued item is now accepted (storing unknown) where the old fall-through return false made GenericFrame skip putScreenValue/refreshFrameWidget and leave the selection untouched.

Valid. Testcase:

def var c as char view-as radio-set radio-buttons "A","a","Nothing","". 
def frame f c. 
display c with frame f. 

c:screen-value = "a".
c:screen-value = "".

4GL and trunk selects the Nothing button, whereas 11191a ends up with A. Reverted the change.

  • [MAJOR] functional GenericFrame.setScreenValue: The same non-parsing branch loses the unknown-value typing, regressing an intentional OE behavior. ComboBoxWidget.resolveUnknownScreenValue (lines 2017-2020) branches on the buffer value's class — bdt.getClass().isAssignableFrom(date.class) ? new character("") : new character() — and GenericFrame line 3662 documents the rule ("date/datetime/datetime-tz all return empty string for unknown", implemented per history entry 048 FER 20250312). Before the change parseValue (3930-3941) short-circuited unknown input to BaseDataTypeFactory.instantiate(type), i.e. an unknown date, so the test passed and the read-back was ""; now the buffer holds an unknown character, the test fails, and the read-back is ?. Trigger (GUI): DEFINE VARIABLE d AS DATE VIEW-AS COMBO-BOX SUBTYPE SIMPLE LIST-ITEMS "01/01/2020". then d:SCREEN-VALUE = ? then read d:SCREEN-VALUE. Scope: confined to plain DATE — for datetime/datetimetz the pre-existing isAssignableFrom test is written backwards and was already false, and for the numeric types both before and after yield unknown.

You can not have SIMPLE/DROP-DOWN combo-boxes with non-character data types, even for dynamic widgets:

def var d as handle.
create combo-box d assign 
    data-type = "date" 
    subtype = "simple" 
    list-items = "10/10/2010".

def frame f.
d:frame = frame f:handle.

enable all with frame f.
wait-for window-close of current-window.

ComboBoxWidget.shouldParseScreenValue() returns false only for GUI combo-boxes with SIMPLE or DROP-DOWN types, which can not have a DATE data type.

  • [MAJOR] functional GenericFrame.convertScreenValue: Letting an unparseable value into the screen buffer (correct per the matrix, "accepted as-is") makes a downstream read silently swallow a raised 4GL error and substitute the type default. ComboBoxWidget.inputValue to GenericWidget.inputValue to frame.getter(id, integer.class, true, false) to convertScreenValue: the (character, String) constructor lookup fails into the blanket catch (Exception exc) at line 8841, then integer.getConstructor(character.class) to newInstance to int64.setValue(String) to NumberType.parseDouble("abc") to errorInvalidChar to recordOrThrowError(76, "Invalid character in numeric input a") — and that ErrorConditionException (wrapped in InvocationTargetException) is discarded by the second blanket catch (Exception exc) at line 8861, leaving value null so defaults true substitutes BaseDataType.generateDefault(integer.class). Trigger (GUI): def var i as int view-as combo-box simple list-items "1","2". def frame f i. i:screen-value = "abc". message i:input-value. prints 0 and error 76 never surfaces — not even in ERROR-STATUS. The blanket catch is pre-existing, but this diff is what makes the path reachable: before the change an unparseable value could never enter the buffer for a GUI SIMPLE/DROP-DOWN combo. Caveat: inputValue deliberately maps an unknown result to generateDefault (history entries 038 SAT 20231025/26), so the returned 0 may be OE-correct; the destroyed error condition is a defect regardless. Worth confirming on OE whether 76 (or 5321) surfaces for INPUT-VALUE of an unparseable integer combo screen value.

Again, you can not create GUI combo-boxes of subtype SIMPLE / DROP-DOWN with non-character modes.

  • [MINOR] style ControlSetEntity.invalidScreenValueWarning: The new protected instance method (line 3291) is placed after the private instance methods controlSetItem/sortItems and the private static method bubbleSort, violating the required member ordering (protected instance methods must precede private static and instance methods). Relocate it up among the other protected instance methods, e.g. near recoverFromParsingError/isValidScreenValue.
  • [MINOR] style RadioSetWidget: The new field comment for savedValue (line 145) uses a plain block comment /* ... */ instead of the javadoc /** ... */ style used for field documentation elsewhere in the package (e.g. every field in GenericWidget). Convert it to /** ... */.

Solved.

Please check rev. 16626.

Also available in: Atom PDF