Implement Responsive Client-Side Autoscaling#90
Implement Responsive Client-Side Autoscaling#90Lightning11wins wants to merge 106 commits intomasterfrom
Conversation
…with an unknown type.
…yout, button, editbox, html, image, label, scrollpane, textbutton, & treeview.
Fixes components, dropdowns, and images. Renames flex variables to be more clear. Send parent_w and parent_h to client.
Add tab-height support to apos.c. Add design support to apos.c. Add IsDesign to the WgtrClientInfo struct. Fix spelling mistakes. Clean up.
Add an error message when cxsecVerifySymbol_n() fails. Improve an existing error message when htr_internal_WriteWgtrProperty() fails to write a property of an unknown type. Set Centrallix event listener to be explicitly non-passive, fixing a console error when later code assumes that calling preventDefault() is allowed.
Fix a bug in qprintf() that caused % and & characters inside conditional printing areas to always print, regardless of the condition. Improve documentation for qpfPrintf_va_internal() and qpf_grow_fn_t(). Clean up.
Add shortcut functions: ht_flex_x(), ht_flex_y(), ht_flex_w(), ht_flex_h(). Rename fl_scale_x (was total_fl_x). Rename fl_scale_y (was total_fl_y). Rename fl_scale_w (was total_fl_w). Rename fl_scale_h (was total_fl_h). Rename fl_parent_w (was parent_w). Rename fl_parent_h (was parent_h). Remove ht_flex_format_all and ht_flex_all(). Improve usage of new feature in previously updated widgets. Fix spelling mistakes. Clean up.
Clean up some apps.
Add Math.clamp() and Math.isBetween(). Add getParentSize(), getParentW(), and getParentH(). Refactor getRelativeX/Y/W/H() to call the new getRelative(). Refactor setRelativeX/Y/W/H() to call the new setRelative(). Add fast_setRelativeX/Y(). Add setResponsiveX/Y/W/H() using a new shared setResponsive(). Add responsiveness to moveTo() with the new functions. Fix style guide mistakes. Clean up.
Add support for more edge cases with undefined values to wgtrGetServerProperty(). Add the Log action (and docs). Add the ReloadPage action (and docs). Improve documentation for the Alert widget.
Add htr_action_point() to ht_render.js to handle the point action responsively. Refactor pane and window widgets to use the htr_action_point(). Improve documentation and code clarity for htutil_point(). Improve documentation for the point action.
Update al_reflow() to create responsive CSS using the setResponsive() functions. Improve documentation for al_childresize(). Optimize and clean up inefficient code.
Rewrite C & JS code to use responsive styling. Fix a bug that allowed multiple tabs to be selected at once. Fix spelling errors. Clean up.
Remove the moveable attribute. Simplify clock widget event code. Clean up cl_get_time().
Add tab_w and tab_h parameters to tab_features.app to fix broken left and right tab locations. Remove unnecessary button widget from autoscale_test.app.
…uced by disabling clipping CSS by default).
Add dragging cursor styles for window widgets. Add cursor styles for the checkbox, radiobutton, scrollpane, treeview, and window widgets. Update existing cursor styles for the button, datetime, dropdown, imagebutton, tab, and textbutton widgets. Add the .dt_dropdown CSS class to make styling the datetime dropdown easier. Update table columns to use col-resize instead of move. Clean up cursor CSS styles to follow consistent code styling.
…atives. Rewrite getters and setters in the ClipObject polyfill to replace __defineGetter__() and __defineSetter__(). Rewrite getters in the datetime, dropdown, and radiobutton widgets to replace __defineGetter__(). Rewrite pg_area() constructor to replace __defineGetter__(). Define a base when parsing relative properties to fix edge cases. Remove unused debug setters in the table widget. Remove unused debug code.
…aster. Replace calls to fast_setRelativeX/Y() with calls to setRelativeX/Y().
Remove obsolete workaround code. Remove a completed todo. Improve a comment.
Fix windows failing to release when the mouseup event comes from outside the web page. Update responsive event listener to use a pattern more similar to other resize code. Add .titlebar to window object. Add global wn_windows array that stores all windows.
Greptile SummaryThis PR implements client-side responsive autoscaling for the Centrallix application server. It introduces flexible layout calculations in Key findings from this review:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| centrallix/htmlgen/htdrv_tab.c | Major refactor of the tab widget renderer; introduces a critical logic bug where page_type is always set to "static" (breaking dynamic tabs), multiple new strcpy violations, absence of goto error pattern, and a config_buf memory management TODO. |
| centrallix/htmlgen/htdrv_scrollpane.c | Large rewrite of the scrollpane renderer; adopts goto err error handling correctly, but has a missing space in "<table" string concatenation in the Netscape DOM branch producing malformed HTML, and a renamed driver name strcpy. |
| centrallix-os/sys/js/ht_geom_dom1html.js | Adds responsive geometry helpers (setResponsive*, getParentW/H, getRelativeW/H); introduces Window.clipped (capital W) bug in the debug getClipped() function which will throw a TypeError at runtime. |
| centrallix/wgtr/apos.c | Extensive comment additions, refactored aposSpaceOutLines to track loc_fl/my_fl flex weights per grid line, and aposSnapWidgetsToGrid extended to write fl_scale_* properties; logic follows existing patterns with thExcessiveRecursion guards preserved. |
| centrallix/htmlgen/ht_render.c | Adds serialization of new fl_scale_* and fl_parent_* widget properties to the client WGTR, modernises HTML output (removes language="javascript", adds charset, lang), fixes bug in htrRender ordering of ns and build_wgtr_ init calls, extends DATA_T_DOUBLE/DATETIME/INTVEC/STRINGVEC property serialization. |
| centrallix/htmlgen/htdrv_table.c | Significant refactor: splits the large tbld_init() call into multiple readable sub-calls, renames tbld%POSpane to tbld%POSbase, adds responsive flex CSS for layout; two new strcpy violations introduced. |
| centrallix/wgtr/wgtr.c | Adds fl_scale_* and fl_parent_* fields to wgtrNewNode initialisation and wgtrGetPropertyType/wgtrGetPropertyValue dispatch; clean, low-risk changes with explicit zero-initialisation. |
| centrallix-os/sys/js/htdrv_scrollpane.js | Complete rewrite of the scrollpane JS; replaces global mutable vars with scoped state, adds wheel/keyboard scrolling, uses a ResizeObserver to keep the scroll thumb in sync; generally well-structured. |
| centrallix/include/wgtr.h | Adds fl_scale_x/y/w/h (double) and fl_parent_w/h (int) fields to WgtrNode for responsive layout calculations, and IsDesign flag to WgtrClientInfo; straightforward header change. |
| centrallix/include/apos.h | Adds loc_fl/my_fl float fields to AposLine, updates aposSnapWidgetsToGrid signature, and adds several readability macros (APOS_IS_BORDER, APOS_VERTICAL, isScrollpane, etc.). |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["Server: wgtrVerify()"] --> B["aposAutoPositionWidgetTree()"]
B --> C["aposBuildGrid() — draw lines & sections"]
C --> D["aposSetFlexibilities() — compute fl_x/y/w/h"]
D --> E["aposSpaceOutLines() — distribute space\n✦ NEW: saves loc_fl / my_fl per line"]
E --> F["aposSnapWidgetsToGrid() — snap widgets to lines\n✦ NEW: writes fl_scale_x/y/w/h to each WgtrNode"]
F --> G["htr_internal_BuildClientWgtr_r()\n✦ NEW: serialises fl_scale_* + fl_parent_* to JS"]
G --> H["Browser receives HTML + WGTR JSON"]
H --> I["startup_() → build_wgtr_()"]
I --> J["setResponsiveX/Y/W/H() in ht_geom_dom1html.js\n✦ NEW: converts server px → % using fl_scale_*"]
J --> K["window resize event"]
K --> L["Recalculate all element positions/sizes\nusing stored fl_scale weights"]
L --> J
Last reviewed commit: c53a8f4
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| node->fl_y = fly; | ||
| node->fl_width = flwidth; | ||
| node->fl_height = flheight; | ||
| node->fl_parent_h = node->fl_parent_w = -1; | ||
| node->ObjSession = s; |
There was a problem hiding this comment.
fl_scale_* fields not initialized in wgtrNewNode
fl_scale_x, fl_scale_y, fl_scale_w, and fl_scale_h are not explicitly set to 0.0 here. fl_parent_w/fl_parent_h received explicit initialisation (= -1) in this same commit, but the fl_scale_* fields did not. If nmMalloc returns unzeroed memory, aposSnapWidgetsToGrid will += random garbage into these fields, silently producing wrong flexibility values for every widget.
Add:
node->fl_scale_x = node->fl_scale_y = node->fl_scale_w = node->fl_scale_h = 0.0;alongside the other new field initialisations.
There was a problem hiding this comment.
@gbeeley I think Greptile is just wrong here. 1585 calls memset(node, 0, sizeof(WgtrNode));, so this data should already be 0. Am I missing something?
I'm still going to make this explicit because A) I think it makes the code more clear and B) the compiler will probably prune this line when optimization is turned on, but I don't think this is really necessary.
Additional Comments (1)
int
ht_get_parent_w__INTERNAL(pWgtrNode widget)
{
if (thExcessiveRecursion())
{
mssError(1, "HTRND", "Resource exhaustion in ht_get_parent_w__INTERNAL");
return 0;
}
const int cached_value = widget->fl_parent_w;
if (cached_value != -1) return cached_value;
const pWgtrNode parent = widget->Parent;
if (!parent) return widget->fl_parent_w = 0;
const int parentWidth = parent->width;
return widget->fl_parent_w = (parentWidth >= 0)
? parentWidth - (parent->left + parent->right)
: ht_get_parent_w__INTERNAL(parent);
}Apply the same fix to Context Used: Rule from Why: This prevents sta... (source) |
Fix critical invalid HTML generation for pgstat. Add null guards and stack exhaustion checks to ht_get_parent_w/h__INTERNAL(). Add explicit initialization for fl_scale_* fields. Fix quotes breaking CSS background-color. Fix incorrect CSS styling. Replace Math.clamp() & Math.isBetween() debug versions with their development versions. Improve naming in thExcessiveRecursion(). Clean up.
|
@greptileai Can you rereview this and let me know if you have any other comments? Did I resolve your issues? |
Add copyright notice to htdrv_button.c. Fix incorrect line wrap in copyright notice for net_http_conn.c.
|
@greptileai Can you rereview this and let me know if you have any other comments? Did I resolve your issues? |
| if (type == NULL || strcmp(type, "dynamic") != 0) strcpy(page_type, "static"); | ||
|
|
There was a problem hiding this comment.
type is always NULL here — dynamic tab pages silently broken
wgtrGetPropertyValue(tab, "type", ..., POD(&page_type)) stores the result into page_type (a char[32]), but the immediately following condition checks type, which is a separate char* variable initialized to NULL at the top of the function and never assigned in this code path.
Because type == NULL is always true, strcpy(page_type, "static") is unconditionally executed regardless of what the tab page's "type" property says. This silently breaks "dynamic" tab pages for every caller.
The old code (pre-PR) correctly used POD(&type) as the out-parameter and checked the return value:
if(wgtrGetPropertyValue(tabpage_obj,"type",DATA_T_STRING,POD(&type)) != 0)
strcpy(page_type,"static");
else if(!strcmp(type,"static") || !strcmp(type,"dynamic"))
strcpy(page_type,type);
else
strcpy(page_type,"static");The same bug appears again at line 607 for the second tab-page rendering loop. Suggested fix for both sites:
if (wgtrGetPropertyValue(tab, "type", DATA_T_STRING, POD(&type)) != 0 ||
(strcmp(type, "static") != 0 && strcmp(type, "dynamic") != 0))
strtcpy(page_type, "static", sizeof(page_type));
else
strtcpy(page_type, type, sizeof(page_type));| .from(Window.clipped) | ||
| .filter(id=>id) |
There was a problem hiding this comment.
Window.clipped (capital W) is undefined — Array.from will throw
Window (capital W) is the Window interface/constructor object, not the current window instance. Window.clipped will be undefined, causing Array.from(undefined) to throw TypeError: undefined is not iterable whenever getClipped() is invoked.
| .from(Window.clipped) | |
| .filter(id=>id) | |
| .from(window.clipped) |
| "<table" | ||
| "border='0' " | ||
| "cellpadding='0' " | ||
| "cellspacing='0' " | ||
| "width='%POSpx' " | ||
| "height='%POSpx' " | ||
| "><tr><td>", | ||
| id, | ||
| w - 2, |
There was a problem hiding this comment.
Missing space after <table produces malformed HTML
C adjacent-string-literal concatenation merges "<table" and "border='0' " into "<tableborder='0' ...", which is invalid HTML. All attributes will be silently ignored by browsers (or worse, cause parse errors) in the Netscape DOM branch.
| "<table" | |
| "border='0' " | |
| "cellpadding='0' " | |
| "cellspacing='0' " | |
| "width='%POSpx' " | |
| "height='%POSpx' " | |
| "><tr><td>", | |
| id, | |
| w - 2, | |
| htrAddBodyItem_va(s, | |
| "<div ID=\"sp%POSarea\">" | |
| "<table " | |
| "border='0' " | |
| "cellpadding='0' " | |
| "cellspacing='0' " | |
| "width='%POSpx' " | |
| "height='%POSpx' " | |
| "><tr><td>", | |
| id, | |
| w - 2, | |
| h - 2 | |
| ); |
1 participant
After extracting changes into #87, #88, #89, and removing a few minor/unnecessary changes; this PR is hopefully slightly more manageable.
GitHub Relationships:
This PR includes several
TODOs:TODOs for Israel to handle after Update Duplicate Detection #77 is merged intomaster.TODOs for Greg (although other reviewers are welcome to handle these if they have the requisite knowledge), in the following files:ht_render.c,htdrv_page.c,htdrv_tab.c, andhtdrv_table.c.There are several known issues with this code:
widget/pane: resize action (testapps/kardia/modules/payroll/pay_form.app)widget/image: offset & scale actions (not used anywhere)widget/objcanvas: add_osrc_object()ht_get_parent_w/h__INTERNAL()&wgtrGetContainerWidth/Height()should be merged.ht_get_parent_h__INTERNAL()(mine) gives a height 2px taller.