JavaScript port: real DOM text, incremental semantics, and native-resolution rendering - #5552
Open
shai-almog wants to merge 126 commits into
Open
JavaScript port: real DOM text, incremental semantics, and native-resolution rendering#5552shai-almog wants to merge 126 commits into
shai-almog wants to merge 126 commits into
Conversation
accessibilityTreeChanged wiped the overlay with setInnerHTML("") and rebuilt
every element, with fresh event listeners, on each invalidation. Because
CHANGE_BOUNDS is raised by every setX/setY/setWidth/setHeight, that ran on every
scroll step and layout pass: a full re-marshal of the tree across the worker
bridge, and -- more importantly -- it discarded DOM focus and any in-progress
text selection each time. Nothing selectable or focusable could live in the
overlay while that was true.
Move the projection into JavaScriptSemanticOverlay, which keys elements by the
stable semantic node id and reuses them across invalidations. Attributes,
geometry, text and child ordering are diffed against retained worker-side state
so only real changes are written, and listeners are bound exactly once per
element. Nodes already in the correct slot are never re-inserted, which is what
keeps a focused or selected element from being moved.
The tree is now walked from getRootIds() through getChildIds() rather than over
the node map's iteration order, so sibling order is defined by the snapshot and
a child can never be visited before its parent exists.
LINK nodes become real <a> elements, for the status-bar URL preview,
middle-click and modifier-click to open in a new tab, and the native context
menu that an ARIA role alone cannot provide.
No DOM property is read back; structure and previously applied values are
tracked in the worker, so every bridge call stays a fire-and-forget write and
the port's no-barrier-reads invariant holds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Canvas text is a bitmap. It cannot be selected or copied, the browser's find-in-page cannot see it, and it rasterizes differently from the browser's own text -- which is most of why the port does not read as a web page. Intercept drawString on its way to the display surface and emit a positioned DOM element instead. Codename One stays the sole layout authority: by the time a run reaches the layer the line is already broken and placed, so each run is a single white-space:pre element at an absolute coordinate and the browser cannot wrap or reflow it. A metrics disagreement between measureText and DOM text layout can therefore only show up as a sub-pixel rendering difference, never as a clipped label. Measurement itself is untouched and stays on the worker's OffscreenCanvas. The interception sits on BufferedGraphics, which is the display graphics. Offscreen surfaces use plain HTML5Graphics, so text painted into a transition buffer, a paint lock image, a ComponentImage or a screenshot still rasterizes onto the bitmap those callers read back. Each run is wrapped in an element clipped to the graphics clip in force when it was drawn, which reproduces canvas behaviour for text in a scrolled container without the layer knowing anything about the component hierarchy. Runs are pooled per component and reused across repaints, since a scrolling list repaints continuously and an element per run per frame would swamp the bridge. Three cases deliberately stay on the canvas: - Cell renderers. A renderer is one component instance stamped at N positions, so runs cannot be keyed by it -- every row would overwrite the previous row's element and only the last would survive. - Anything outside the displayed form. The layer sits above the canvas as a whole, so nothing drawn on the canvas afterwards can occlude it; a modal dialog paints the form beneath it as its backdrop, and promoting that text would float it over the dialog. - Shape clips and non-identity transforms, which have no faithful CSS equivalent here. Bitmap fonts need no exclusion: Graphics.drawString renders a CustomFont itself and never reaches the implementation. The layer is aria-hidden so assistive technology keeps reading the semantic overlay and no label is announced twice, and it takes no pointer events so the canvas keeps ownership of hit testing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three features were written when the port ran on the main thread and were never adjusted for the worker, where the objects they reach for do not exist. Each one failed silently. History. pushHistoryState() was a @JSBody, so it was compiled into the worker and threw on every form change -- the port logged "history.pushState not supported. Back command will not work." on each navigation, and it was right. Route the call through the window binding so it lands on the main thread, and add a popstate listener that dispatches the current form's back command, so the browser's Back button and the in-app back control finally agree. A popped entry is replaced afterwards, or the second Back would leave the app while there was still somewhere to go inside it. Media queries. Dark mode, reduced motion, forced colors, increased contrast and reduced transparency were all evaluated with window.matchMedia inside a @JSBody. The worker has no matchMedia, so every one of them answered false: dark mode was never detected on any browser. Evaluate them through a MediaQueryList binding on the main thread, cache the result rather than re-reading it per call, and refresh the cache from a change listener so switching the OS theme takes effect without a reload. Cursors. Every Codename One cursor is already mapped to its CSS equivalent and wired to the hover path, but isSetCursorSupported() was never overridden, so the documented capability check reported false to applications. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Canvas rendering could only ever be checked by comparing pixels. With text and semantics in the DOM they can be asserted directly, which is both faster and diagnostic -- a failure names what is wrong instead of reporting a pixel delta. verify-javascript-web-overlay.mjs drives a built bundle and checks that text is promoted to real DOM text, that the text layer is aria-hidden and takes no pointer events, that the canvas is hidden from assistive technology, that semantic nodes keep their identity across invalidations, and that history entries are pushed on navigation. The identity check tags elements with a JavaScript property and re-reads them after further invalidations: a rebuild would drop the tag, which is exactly the regression that would silently take focus and text selection with it. STATUS.md now describes the two layers, the cases that deliberately stay on the canvas, and the remaining gaps -- drag-selection, sub-pixel vertical placement, and occlusion within a single form. It also records that the screenshot goldens still need to move to page.screenshot() and be rebaselined, since a canvas capture no longer contains the text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codename One addresses device pixels. The iOS port detects the retina factor and
multiplies/divides the values it hands the native primitives, so the framework
draws at the display's real resolution. This port pinned the ratio to 1 instead
and described that as working "in CSS pixels end-to-end", with ?pixelRatio=2 as
an opt-in.
That is a misreading of the API, and it costs image quality everywhere: with the
backing store equal to the CSS size, every HiDPI display renders a 1x bitmap that
the browser then upscales 2x or 3x. Text shows it first, because a stretched
glyph is obvious next to a browser-rendered one, but it softens everything.
Report window.devicePixelRatio instead. scaleCoord/unscaleCoord already convert
at the DOM boundary -- peers, overlays, pointer coordinates -- and nowhere else,
which is the same split the iOS port uses, so nothing above the boundary changes.
?pixelRatio=N still pins a factor for the screenshot harness and skin designer.
The duplicate default in the @JSBody fallback is updated too, so the two cannot
disagree.
The backing store is also rounded rather than truncated: at a fractional ratio
such as 1.5 a truncated store no longer equals cssSize * ratio, so the browser
rescales by a hair and softens every glyph edge again.
Verified in a browser at deviceScaleFactor 2: the canvas is now 750x1334 backing
behind a 375x667 CSS box, and the semantic overlay reports correctly converted
CSS-pixel bounds.
Also fixes a CSS ordering bug in the text layer: the font shorthand carries its
own line-height ("18.9px/1.0"), so emitting it after the explicit line-height
reset the line box the run was supposed to use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TextField and TextArea edit through a real native input positioned over the component, the same shape the Android, iOS and JavaSE ports use. The element was carrying almost none of the metadata a browser needs to treat it as a form field, so autofill and password managers had nothing to work with and mobile keyboards could not specialise. The constraint switch was also wrong. A constraint is a base type in the low bits with flags above it -- PASSWORD is 0x10000 -- but the code compared the whole value, so a field declared PASSWORD | EMAILADDR matched no case and was edited as clear text. Mask the base and test PASSWORD as the flag it is. Beyond the type, the element now carries what browsers actually read: inputmode selects the on-screen keyboard, autocomplete is what lets a password manager or address autofill offer a value, and autocapitalize/spellcheck reproduce what INITIAL_CAPS_*, SENSITIVE and NON_PREDICTIVE already mean on a native platform. A component name becomes the field name so a manager can pair a username with a password instead of seeing unrelated edits. Applications can override the autocomplete token -- to separate a sign-in field from a change-password field -- with the cn1$autocomplete client property. The dormant TextAreaNativeOverlay path shares the same helper so the two cannot drift. Verified against a real field: autocomplete is now present where the element previously had no autofill metadata at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The app captures its own screenshot by reading back the canvas and ships the bytes over the shared cn1ss WebSocket transport. That readback is no longer the whole picture: text is now promoted into a DOM layer above the canvas, so a canvas-only capture is missing every label on screen. The transport is shared with the iOS, Android, watch and TV jobs, so it should not be forked for one platform. Take the authoritative capture in the JavaScript-specific Playwright harness instead -- it screenshots the composited page, canvas and overlays together -- and write it over the app's file in the same directory. The delivered artifacts and the golden comparison are unchanged. Captures are queued the moment the app reports its own, since the suite has not advanced at that point and the page still shows the frame under test, and are serialized and drained before teardown so the final test of a run does not lose its screenshot and read as a golden miss. A capture failure leaves the app's canvas-only PNG in place rather than dropping the test. Verified locally: 28 composited 750x1334 captures where the canvas-only path produced 375x667. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ffdd8b2e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
Contributor
Cloudflare Preview
|
Three defects from review, all real. Capture raced the suite. The composited screenshot was taken when a console marker appeared, but that marker is emitted before the WebSocket send and the suite advances from its own ACK, so nothing held the app still: a screenshot could land after the next test had drawn its form and be written under the previous test's name. Drive the capture from a hook the page calls instead -- browser_bridge.js awaits __cn1CompositeCapture inside the screenshot host call, and the worker is blocked on that call, so the suite genuinely cannot advance while the screenshot is taken. The test name now comes from the suite's own "starting test=" marker rather than being parsed out of the capture line. Paint hooks fired for offscreen renders. beforeComponentPaint/afterComponentPaint run for every paint, including Component.toImage(), ComponentImage, paint locks and drag images. Those draw through a plain HTML5Graphics, so no run is promoted -- which meant the frame opened and closed around them saw zero runs and released the component's real on-screen DOM text. Creating a drag image could blank the labels underneath it until the next repaint. Only open a frame when the graphics is the display graphics. Hidden components kept their runs. The synchronization pass only asked whether a component was still on the displayed form. Hiding a component, or any ancestor of it, stops it painting without detaching it, so its pooled runs were never refreshed and never released -- they stayed above the canvas after the parent's repaint had already cleared the pixels beneath them. Retention now also requires the component to be displayable. Verified locally with the bridge hot-swapped into a built bundle: the hook installs, captures are page-driven, and test names track the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e157713e1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Second round of review, three defects. The composited PNG was being overwritten. The harness wrote it into the cn1ss directory while the screenshot host call was still running; that call then returned the canvas-only data URL, the app sent those bytes over the WebSocket, and the server wrote them over the composited file. Hand the image back through the hook instead and return it as the screenshot, so the bytes travel the normal path and the cn1ss server stays the only writer of the PNG. There is no longer any ordering to get wrong, and the harness no longer needs the output directory or the test name at all. The glass pane and a dragged component paint on the canvas after the children, so they cannot cover promoted text -- the layer is above the canvas as a whole. Suspend promotion while a form has either, which puts the text back on the canvas where paint order still decides what is on top. This is the same reasoning already applied to modal dialogs, extended to the in-form cases. Back reached the root form and then did nothing. Popping an entry and going back one form already keeps history depth and navigation depth in step, because every form show pushes an entry; pushing a replacement after handling a pop left a dead entry, so from the root form the first Back was swallowed and only the second left the app. Drop the replacement push. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94f5e7209e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
Collaborator
Author
|
Compared 149 screenshots: 149 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. |
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark ResultsDetailed Performance Metrics
|
… DOM text Regenerated from the CI run's delivered artifacts rather than locally, so the fonts and rasterization match the environment the comparison runs in. Two things changed at once. The port now renders at the display's real pixel ratio, so captures are 750x1334 device pixels instead of a 375x667 1x surface the browser used to upscale. And text is promoted into a DOM layer, so the authoritative capture is a screenshot of the composited page rather than a canvas readback. Images the app renders offscreen and delivers itself -- the per-appearance theme captures, for instance -- are unaffected by the second change and still carry their text, because an offscreen paint goes through plain HTML5Graphics and never promotes. 180 of the 181 goldens are replaced. ToastBarTopPosition is not delivered by the suite (a parked test) so its golden is left alone rather than being deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7fcccf5934
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 143 screenshots: 143 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 144 screenshots: 144 matched. |
The screenshot rebaseline landed 180 of 181 goldens byte-identical, but ToastBarTopPosition stopped delivering: "timeout waiting for DONE stage=capture-requested". The suite blocks on the capture host call, which now awaits the composited screenshot, and `animations: 'disabled'` makes Playwright wait for animations to settle -- a screen with a running toast never settles, so the capture outlived the suite's own timeout. Drop that option and bound the screenshot with an explicit timeout and a race, so this call always resolves promptly. Falling back to the canvas readback costs the DOM text in one golden; hanging costs the whole test. Suspension was decided too late. It was applied while flushing, by which point the components had already painted under the old setting -- their strings either promoted and left off the canvas, or rasterized onto it -- so flipping the flag afterwards left the frame either missing its text or showing it twice. Decide it at the start of the frame instead, before any component paints. Paint locking is off for this port. A locked component serves a cached image and returns from paintInternal() before the per-component hooks run, so it stops reporting text while its DOM runs stay on screen: the image's rasterized text and the live DOM text are then both visible. The two representations cannot be reconciled and the lock is only an optimisation, so it is disabled. The semantic overlay wiped its own children. setTextContent replaces every child, so relabelling a STATIC_TEXT or HEADING node detached its semantic children and custom-action buttons while the retained ordering still claimed they were attached -- the reconcile pass then skipped re-adding them and those controls disappeared until an unrelated structural change rebuilt them. The label now lives in a child of its own. The root form pushed a dead history entry. It has nothing behind it, so the first Back popped that entry without navigating and leaving the app from the root took two presses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f210351d0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Dropping `animations: 'disabled'` from the capture was wrong. Playwright uses it to settle animations and wait for fonts, and without it the screenshot races the canvas presentation: 59 of 181 goldens went from equal to different, the static graphics and chart tests among them. Restore it. The timeout and race stay, so a screen that never settles still cannot stall the suite -- it falls back to the canvas readback, which costs the promoted text in that one golden rather than the whole test. SENSITIVE now beats PASSWORD when choosing autocomplete. A field marked both was getting "current-password", which is an explicit invitation to offer a stored value, while SENSITIVE asks that it never be retained for completing schemes. Custom accessibility actions refresh their label. Replacing an action with the same id and new wording -- an Expand that becomes a Collapse -- left the retained button announcing the old text until the action was removed entirely. Buffered transitions suspend the text layer again. A fade renders only its prebuilt images and never puts a component through the display graphics, so moving the suspension decision to the start of a component paint meant it never ran during one: the outgoing form's DOM text stayed fixed above the animation for its whole duration. The drain checks it too, and only ever suspends there -- resuming stays at the start of a frame, before any component paints. Two history corrections. An in-app back command reaches setCurrentForm looking exactly like forward navigation, so it pushed an entry instead of spending one; returning to the form we came from is now recognised as backward. And a pop guard that refuses a back command leaves the same form showing after the entry has already been consumed, so the entry is put back rather than letting the next Back leave the app without asking the guard again. Where no back command exists at all, Back now keeps unwinding instead of being swallowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 175a9b204b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Collaborator
Author
|
Compared 217 screenshots: 217 matched. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 898aadc2a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both tests were asked about the whole run, so a clip whose bounds fall short of the glyphs still had the draw's outline consulted about text it could not touch. Each is now asked about the part of the run the clip's bounds reach, and a clip that reaches none of it answers on its own. What remains is that both can answer yes about one rectangle while touching different parts of it. The exact answer is a polygon intersection, and the code comment records why it is not attempted: getting it wrong the other way leaves glyphs above a draw that covered them, which is wrong on screen, while answering too generously only sends a component's text back to the canvas. The 52 screenshots the clip and transform tests deliver are pixel-for-pixel identical to the run before this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38ad81c878
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The actions region is created the first time a node needs a control, which happens while the tree is still being walked -- before the roots are attached -- so it stayed ahead of the whole form in document order. A keyboard or screen-reader user met "Set text" and "Delete" before reaching the field or item that gives them their meaning. It is moved behind the roots each time the tree is reconciled. Verified on the TextFieldTheme screen: the region is the last child of the overlay, with its three controls after the form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Captures asked for while one was waiting for the rasterizing repaint were drained after the leading callback ran, so a callback that threw took the rest of the list with it: the flag that sends a capture to the queue was already down, so every later capture took the immediate path and walked past the ones still standing there. The list is taken and emptied before any of them run, and each runs on its own. These callbacks belong to unrelated callers that happened to ask during the same frame, so one failing is no reason for the others to go unanswered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30fafdeac2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A clip stays fixed in the coordinates it was installed in while a draw is in the coordinates in force now, and the two were intersected as though they shared a space. Clip screen x=0..50, translate by 100, then fill x=-100..50: the canvas paints screen x=0..50 and the report landed on screen x=100..150, so text the fill really did cover was left above it -- the mistake that shows on screen rather than the one that only sends text back to the canvas. Both are now projected through their own transform and intersected on screen, which is where the canvas resolves them. The clip and transform screenshots are pixel-for-pixel identical across the change: 53 tests, no differences. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 437089f5a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The coverage test walked every outline as a closed loop, so a partial drawArc or an open subpath of drawShape was given a chord across its ends that the canvas never strokes. Text beside the curve was detached for crossing a line that was not there, and a detached component stays on the canvas for the rest of the form. An outline now says whether it closes by repeating its first point -- a rectangle, a polygon, a rounded rectangle, a wedge, a whole ellipse and a subpath that ends in SEG_CLOSE all do; a partial arc and an open subpath do not. The segment walk follows the points it is given, and only a fill adds the closing edge, because context.fill() really does paint it. Checked both ways on a 90-degree arc: a rectangle on the chord between its ends is no longer reported, one on the curve still is, and a stroked rectangle keeps its closing edge. The arc and clip screenshots are unchanged: 64 tests, no pixel differences. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every keystroke in a SET_TEXT control has already reached the framework, so when focus leaves, an obscured field's control has nothing left to hold -- and what it held was a secret. type="password" only stops it being read off the screen; the value stayed in the DOM for a script or an inspector, and the sync pass would not clear it, because it leaves a masked field alone precisely so it never writes the secret back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ddd6b0ec2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The port claimed the entry its first form appeared on by replacing that entry's state. The state there is not the port's to take: a page that embeds the canvas keeps its own router data on it, and a host that finds its state replaced loses the navigation it was in the middle of. Nothing is written now. The entry is recognised instead: an id above everything this session has pushed cannot be one of ours, because a traversal can only reach entries the port created, so it is the entry the page was loaded on carrying an id a previous life of the application left there. It is read as that rather than as a step forward, which is what the rewrite was for. verify-javascript-web-overlay.mjs now counts calls to history.replaceState from the moment the page loads: 1 with the build that rewrote the entry, 0 with this one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A capture that turns the text layer off has to wait for the frame that puts the text back on the canvas, and that wait counted event-thread hops: two, on the reasoning that the first lands before the repaint and the second after it. A repaint is not one flush. This application's image grid paints over several, so the read could land between them and come back with the last panels blank -- graphics-draw-image-rect failed exactly that way twice in CI, both times with byte-identical output, while the same test renders identically on either side of the change locally. The read now waits for the flushes to stop rather than for a fixed number of hops, bounded so a form that never stops flushing still gets its answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3942f28e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The control carried no maxlength, so it was the long way round an application's limit -- and not only for one edit: TextArea.setText() raises maxSize to fit whatever it is given, so text typed past the limit through this control moved the limit for good. The control now takes its length from the field it writes to, and a field with no limit has the attribute removed rather than keeping one it no longer asks for. Verified on the TextFieldTheme screen: both inputs and the textarea come up with maxlength=124, the fields' own maximum. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5360177503
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The limit was applied on the way past two early returns, so a masked field's control -- and one being typed into -- kept whatever limit it was built with. An application that lowers setMaxSize() while a password field is on screen would have left that control accepting the old length, and a value typed past the new limit raises maxSize again on the way in, which is the same bypass one step removed. It is applied first now, before anything that returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bff53963a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… the alpha Two of the same faults the arc had, in the executable that fillRadialGradient uses. It swept the long way round, so fillRadialGradient(.., 20, 200) painted the other 160 degrees -- the wedge came out mirrored against the Android golden, and text in the part it really painted kept the DOM. And its setGlobalAlpha call is commented out, so it paints whatever the graphics alpha is, while coverage was skipping the report when that alpha was zero: text left above pixels the fill had in fact replaced. The sweep now follows the sign of the arcAngle, as FillArc does. Coverage reports this fill without consulting the alpha, because the renderer does not consult it either -- what the canvas does is what coverage has to describe. graphics-draw-gradient changes with the sweep and is rebaselined from CI next. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da70c059d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more places where coverage and the canvas disagreed. DrawCanvas is built with alpha 255 and blits the GPU surface opaquely, but the report went through the guard that drops a fully transparent draw, so a composite made under a transparent graphics left text above pixels it had replaced. And a fillArc or drawArc of zero degrees paints nothing at all, while the coverage outline was the line from the centre out to the rim -- text across that line was taken off the layer, for good, by a draw that touched no pixel. The blit is counted whatever the alpha is; a zero sweep is not counted at all. The 65 screenshots the arc and clip tests deliver are unchanged apart from the gradient this branch already rebaselines. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The golden recorded the complement the partial radial gradient used to paint. Taken from the CI run for da70c05, where it was the only test that differed: pass=168, fail=1, and the wedge now matches the Android golden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 691c44f339
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
History state is any structured-cloneable value, so a page embedding this canvas is free to keep its own router object on an entry. Binding getState as String put that value through a conversion it cannot satisfy, when all the port ever needs is to recognise that the value is not one of its own stamps. Widen the binding to Object and let parseHistoryIndex inspect it as a string only when it actually is one -- anything else reads as foreign, which is exactly how a host page's entry should read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54459a3883
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Editing through the accessibility overlay's SET_TEXT control is still editing that field, but the control was built as a plain type="text" with no inputmode, autocomplete, autocapitalize, spellcheck or name. A screen reader user editing an email, numeric, phone or URL field got the wrong keyboard, and -- worse -- a field marked SENSITIVE or NON_PREDICTIVE had prediction and autofill enabled on the one surface that could type into it. The editor's applyInputConstraints already resolves all of this, so it is shared rather than restated: a constraint added to one now reaches both. Whether to write a type is passed in, because the control's shape is this class's decision -- an obscured field gets a masking single-line input whether or not the component is multiline -- and masking stays with the snapshot's obscured flag, which an application can set either way. Refreshed on every sync, so a constraint changed while the field is on screen is not left behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A field that exposes SET_TEXT was reachable twice over: its semantic element carried tabindex=0 from isFocusable(), and the control that actually edits it is separately focusable. The first stop is the useless one -- the input handlers live on the control, so a keyboard or screen reader user landed on a textbox that announced the field and then refused every keystroke, before reaching the one that works. The semantic owner now yields its stop to the control that represents it, decided from the snapshot rather than from the entry's controls because the controls are built after the attributes are applied. The verify script asserts no field is tabbable alongside its control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 454fafee82
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Now that the field itself is no longer a tab stop, its SET_TEXT control is the only way a keyboard or screen-reader user reaches it -- and the control recorded the editing marker without ever dispatching the node's FOCUS action, which the semantic element it replaced did dispatch. The browser therefore edited the field while Codename One still considered the previously focused component active, so focus styling, focus listeners and any keyboard navigation from there acted on the wrong component. FOCUS is added to every focusable node by AccessibilityManager and its handler calls requestFocus, so dispatching it is what moves the framework's own focus. Recorded before the dispatch, matching the semantic element's listener: the snapshot that comes back reporting the node focused then finds the overlay already agreeing and does not pull DOM focus onto the semantic element, which would take it off the control mid-edit. Focus in the other direction follows the control too -- applyFocus targets it rather than the element, and losing framework focus blurs it -- because the element it would otherwise focus is the half that cannot be typed into. Verified by construction rather than by a behavioural A/B: the framework's focus state is not mirrored into the DOM and the canvas is not stable between frames, so neither is observable from the browser harness. The overlay suite stays at 14/14 with no page errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
Makes the JavaScript port feel like a web page without replacing the renderer.
The obvious approach -- emit DOM per component -- is the one Flutter and Uno both tried and reversed. What they kept is the useful half: canvas for pixels, a targeted DOM layer for text, semantics and input. Flutter's canvas apps ship
<flt-semantics>for ARIA and<flt-text-editing-host>for a real positioned<input>. This PR takes the port to that shape.The layers
Two DOM layers sit above the canvas, which is already
role=presentation/aria-hidden:#cn1-text-layercarries the visible text.BufferedGraphics.drawStringhands each run to the layer instead of the canvas, so what the user sees is real text -- selectable, findable, rasterized by the browser. Codename One stays the sole layout authority: a run arrives already broken and placed, so it is onewhite-space:preelement at an absolute coordinate and the browser cannot wrap or reflow it. Text measurement is untouched and stays on the worker'sOffscreenCanvas.#cn1-accessibility-treecarries the ARIA projection, now updated incrementally.Text that stays on the canvas by design: offscreen targets (transition buffers,
paintLock,ComponentImage,screenshot-- a structural gate, since those use plainHTML5Graphics), cell renderers (one instance stamped at N positions cannot key a pooled element), anything outside the displayed form (a modal dialog paints the form beneath it as its backdrop), shape clips and non-identity transforms. Bitmap fonts need no exclusion --Graphics.drawStringrenders aCustomFontitself and never reaches the implementation.Bugs fixed along the way
The overlay was rebuilt on every invalidation.
accessibilityTreeChangeddidsetInnerHTML("")and recreated every element with fresh listeners.CHANGE_BOUNDSis raised by everysetX/setY/setWidth/setHeight, so this ran on every scroll step and discarded DOM focus and any in-progress text selection each time. Nothing selectable could live there until it diffed.The port rendered at 1x on every HiDPI display.
port.jspinneddevicePixelRatioto 1 and called it working "in CSS pixels end-to-end". Codename One addresses device pixels -- the iOS port detects the retina factor and scales the values it hands the native primitives -- so this was a misreading of the API, and it meant the browser upscaled a 1x bitmap on every retina screen. Text showed it first. Now 750x1334 backing behind a 375x667 CSS box at dpr 2.A password field could be edited in clear text. The constraint switch compared the whole value, but
PASSWORDis a flag (0x10000), soPASSWORD | EMAILADDRmatched no case and fell through totype="text".Three worker/main-thread failures. Written when the port ran on the main thread, silently broken once it moved into a worker:
history.pushStatewas a@JSBodyand threw on every form change (the port logged that the back command would not work); everymatchMediaquery answeredfalse, so dark mode was never detected on any browser, nor reduced motion or forced colors;isSetCursorSupported()reported false although every cursor is mapped and wired.The rule worth remembering: a
@JSBodyruns in the worker. Anything touchingwindow,document,history,matchMediaornavigatorneeds a host binding.Editing
TextArea/TextFieldedit through a real native input positioned over the component -- the Android/iOS/JavaSE shape -- and that path is unchanged apart from the constraint fix and the metadata browsers actually read:inputmodefor the on-screen keyboard,autocompletefor password managers and autofill,autocapitalize/spellcheckmapped fromINITIAL_CAPS_*,SENSITIVEandNON_PREDICTIVE, and a fieldnamefrom the component name.cn1$autocompleteoverrides the token. The newerEditField/EditorViewpath (startTextInput, lightweight with native IME) is untouched.Goldens
Canvas-only captures no longer contain the text, so the Playwright harness now takes the authoritative screenshot of the composited page and writes it over the app's file in the same directory. The shared cn1ss transport used by iOS/Android/watch/TV is not forked. Goldens are regenerated from this run's artifacts -- they change size (750x1334) and gain real text.
Verification
scripts/verify-javascript-web-overlay.mjsasserts the layers directly, which canvas rendering never allowed. Passing 10/10 against a real build at dpr 2, including a check that semantic elements keep their identity across invalidations -- it tags them with a JavaScript property and re-reads it, so a rebuild would fail it.Known gaps, deliberately left
fontHeight()as the line box, which matches CN1's own layout metric but is approximate against browser font metrics to about a pixel.Sheetover text in the same form) is not handled; only cross-form occlusion is.🤖 Generated with Claude Code