Bug #11303
Editor isn't drawn when it is defined using INNER-CHARS and INNER-LINES
100%
History
#1 Updated by Delia Mitric 8 months ago
Testcase:
DEFINE VARIABLE edDesc AS CHARACTER VIEW-AS EDITOR
INNER-CHARS 30 INNER-LINES 3 NO-UNDO font 0.
DEFINE FRAME F-Main
edDesc AT ROW 2.52 COL 15
WITH 1 DOWN SIDE-LABELS three-d
SIZE 80 BY 10.
ENABLE ALL WITH FRAME F-Main.
wait-for close of current-window.
OE: 
FWD: 
#3 Updated by Razvan-Nicolae Chichirau 4 months ago
- Status changed from New to WIP
- Assignee set to Razvan-Nicolae Chichirau
#4 Updated by Razvan-Nicolae Chichirau 4 months ago
- Status changed from WIP to Review
- % Done changed from 0 to 100
- reviewer Hynek Cihlar added
The problem is caused by the order of operations in EditorGuiImpl.initialize(). The scroll pane is initialized and laid out before the editor size is adjusted. Since the editor dimensions are not set yet, the scroll pane inherits a 0x0 dimension. This is considered a valid size, so it does not fall back to the parent dimensions during layout.
The fix is to correct the initialization order: first initialize the scroll pane, then adjust the editor size, and finally lay out the scroll pane.
Hynek: Please review 11303a/rev. 16644.
#5 Updated by Hynek Cihlar about 1 month ago
- Status changed from Review to WIP
- % Done changed from 100 to 90
Code review 11303a revisions 16643..16644
The changes look good to me. Please check the single minor point below.
- [MINOR] functional
EditorGuiImpl.adjustSize: On the INNER-CHARS/INNER-LINES path the 3D inset is not applied, so the editor is laid out ~2 px shorter than it should be inside a THREE-D frame. Duringinitialize()the widget is still parentless (WidgetRegistry.reconstructWidgetcallsinitialize()beforeaddWidgetsToFrame), soisThreeD()returnsfalseand line 2100 usestopInset = 1instead of2-- inconsistent with the rule the same file applies inadjustInnerLines()and in theancestorChangedlistener (change 088 / #8835). The correctiveancestorChangedlistener is gated onconfig.heightPixels > 0, andheightPixelsis only written byBaseEntity.setHeightPixelsWorkerfrom HEIGHT-PIXELS/SIZE-PIXELS, so it stays 0 for this issue's own reproducer (INNER-CHARS 30 INNER-LINES 3in aWITH ... THREE-Dframe) and the listener body never runs. No later path re-derives it either:afterConfigUpdate()only callsadjustSize(false, true, false)wheninnerLines/innerCharsactually change, and by thenconfig.heightCharsis non-zero soexplicitSizeSetupis true and the explicit branch re-derives from the already-wrong value. This is a pre-existing sizing inaccuracy, not a regression of this patch --adjustSize(true)ran equally parentless before the reorder -- but the fix makes it observable for the first time, since the editor is now drawn at all. Recommend a screenshot comparison against OE for the issue's reproducer before closing, and consider relaxing the listener gate to also handleheightPixels == 0(e.g. re-runadjustSize(false, true, true)on ancestor change).
#6 Updated by Razvan-Nicolae Chichirau about 1 month ago
- % Done changed from 90 to 100
- Status changed from WIP to Review
The minor point does not hold. initialize() does run parentless and uses topInset = 1. However, config.heightPixels is already non-zero (45 in my test) after that initial adjustSize(true) because of the aspect work from SyncCoordinatesAspect.afterSetHeightChars() triggered by the following code:
if (forceEditorChars || config.heightChars == 0.0 || ancestorChanged)
{
config.heightChars = inCharHeight;
}, so the
ancestorChanged gate passes when addWidgetsToFrame attaches the editor, and the listener re-derives the height with isThreeD() = true (45 -> 47 px). The result matches OE: +2 px in THREE-D frames for static, dynamic and runtime-changed editors. Measured with the help of height attributes and pixel measurements.
As such, if there are no concerns left, I will start testing.
#7 Updated by Hynek Cihlar about 1 month ago
- Status changed from Review to Internal Test
Razvan-Nicolae Chichirau wrote:
As such, if there are no concerns left, I will start testing.
Please go ahead.
#8 Updated by Razvan-Nicolae Chichirau 1 day ago
- Unit-tests from a customer app
- Editor tests from
testcases - Hotel GUI + 2 customer app smoke-tests
- Tests from #11303-1
Changes only in EditorGuiImpl, no ChUI regression tests needed. If this is enough, 11303a can be merged.
#9 Updated by Radu Apetrii 1 day ago
Alex, is the trunk still frozen for the delivery?
#10 Updated by Radu Apetrii about 7 hours ago
- Status changed from Internal Test to Merge Pending
11303a can be merged right now. I received the confirmation.
#11 Updated by Razvan-Nicolae Chichirau about 7 hours ago
- Status changed from Merge Pending to Test
Branch 11303a was merged into trunk as rev. 16827 and archived.