JS port: let Display.execute navigate the current page instead of prompting - #5483
Merged
Conversation
…mpting
Opening a link from the JavaScript port went through a confirmation Sheet
whenever the browser no longer saw a live user gesture -- which is almost
always, since Codename One dispatches events on its own EDT. The Sheet was
also unusable: it put the buttons EAST of a SpanLabel holding the full URL,
and a URL has no spaces to wrap on, so the label's preferred width pushed
the buttons off screen where they could not be tapped. Its OK button then
called CN.execute() recursively, which with no gesture registered re-entered
execute() and could show the Sheet again indefinitely.
Add a javascript.execute.target property with three values:
auto the new default. Opens a new tab while the page still has user
activation and navigates the current page otherwise, so no Sheet
is ever needed.
_blank new tab only, with the Sheet as the fallback (the old behavior).
_self always navigate the page the app runs in.
Every URL-opening path also called window.open() from inside the Web Worker
the app runs in, where window.open does not exist, so even the backside-hook
path could not have worked. Route them through eval_(), which port.js already
forwards to the __cn1_eval_on_main__ host bridge -- no new host handler is
needed and a page built against an older bridge degrades to the Sheet rather
than failing. Drop the now-dead windowOpen native.
Both Sheets now share createConfirmationSheet(), which stacks the message,
the shortened URL and the buttons in a BoxLayout.y() so nothing can push the
buttons out of reach, and the OK button opens the URL directly instead of
recursing.
Document the property and the reason the gesture is lost on Display.execute
and CN.execute, and in the developer guide's "Playing media and opening
links" section, whose opening paragraph still claimed links always prompt.
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: 1887867500
ℹ️ 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 Sheet's OK callback runs on the Codename One EDT, by which point the browser no longer counts the tap as an ongoing gesture -- so opening from there could be discarded by a popup blocker, which is exactly the situation the Sheet exists to recover from. The OK tap does install a backside hook (pointer handling calls installBacksideHooksInUserInteraction), so queue the open on it and let the drain perform it inside the gesture, matching the download confirmation callback. This also restores what the removed CN.execute() recursion was doing, without its risk of re-entering execute() and re-showing the Sheet. 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: 165d4487b1
ℹ️ 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".
Three fixes from review: Assert page context in the injected script. When the host bundle predates the __cn1_eval_on_main__ handler, port.js degrades to evaluating the script inside the worker. Opening fails loudly there, but a same-window navigation would not: assigning to the worker's read-only location is a silent no-op in non-strict eval code, so execute() would look like it succeeded and do nothing. Every variant now starts with a document check so the worker case throws and the caller can react. Degrade _self on that failure. The _self branch ignored the return value and returned, leaving execute() with no effect. It now falls through to the same backside-hook / confirmation-Sheet path auto already used. Keep failed local downloads off the target policy. A local path whose blob setup failed -- exists() false, or openFileAsBlob() throwing, which the downloadBytesAsFile comment records as easy to hit on this port -- was being handed to openExternalUrl(). Under the auto default that pointed the page at a storage path the server does not serve, unloading the app instead of downloading. Only genuinely navigable schemes now take that path, via a new isExternalUrl(); everything else keeps the previous best-effort new window. That predicate also fixes a duplicated "http:" test that was meant to be "https:". 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: 69d6f0b5cf
ℹ️ 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
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates the JavaScript port’s URL-opening behavior to avoid gesture-related confirmation prompts by default, routing URL opens through the main-thread eval bridge and adding documentation for the new javascript.execute.target setting.
Changes:
- Add
javascript.execute.targetwithautodefault to choose between opening a new tab vs navigating the current page. - Rework JS port URL-opening to use
eval_()(main-thread bridge), remove the deadwindowOpennative, and fix confirmation sheet layout/recursion. - Document the behavior change in the developer guide and
Display.execute/CN.executedocs, including a new snippet fixture.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/developer-guide/Working-With-Javascript.asciidoc | Adds guidance and a new section/table explaining javascript.execute.target behavior. |
| docs/demos/common/src/main/java/com/codenameone/developerguide/snippets/generated/WorkingWithJavascriptJava009Snippet.java | Adds a compilable guide snippet demonstrating the new property values. |
| Ports/JavaScriptPort/src/main/java/com/codename1/impl/html5/HTML5Implementation.java | Implements execute.target logic, routes URL opens via main-thread eval bridge, and refactors confirmation sheets. |
| CodenameOne/src/com/codename1/ui/Display.java | Documents JS-port-specific behavior and property values for Display.execute(). |
| CodenameOne/src/com/codename1/ui/CN.java | Cross-references Display.execute() docs for JS port behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The previous allowlist (http/https/mailto/tel/sms) demoted custom deep links such as the imdb:///find example in Display.execute's own javadoc, along with uppercase schemes and protocol-relative URLs, to a bare window.open() that skipped the gesture-aware flow and ignored an explicit _self target. Before this branch those URLs went through the backside-hook / confirmation path. Classify by structure instead: anything carrying an RFC 3986 scheme is external, whatever its case, as is a protocol-relative //host/path. file: is the exception, since like the bare paths that carry no scheme at all it names local content -- which is what keeps failed local downloads off the target policy. A single character before the colon is read as a Windows drive letter rather than a scheme, and a path that merely contains a colon still fails the scheme grammar. data: is now covered by the predicate, so drop the redundant second test at the top of execute(); its own branch still runs first and downloads it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review nits. Replace the 40 / 37 literals in shortenUrlForDisplay with MAX_DISPLAY_URL_LENGTH and DISPLAY_URL_ELLIPSIS, deriving the truncation point from the marker's length rather than restating it. Verified behavior-identical across the empty, short, long-path and long-host cases. Also report the one dead end the Sheet cannot recover from: when the user confirms and openUrlOnMainThread still fails, the host bundle has no eval-on-main handler, so the worker has no page-side channel at all and re-showing the Sheet would loop. The result was discarded silently; log it instead so it is diagnosable. 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: 84c0423c78
ℹ️ 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".
isBacksideHookAvailable() is true whenever EITHER a gesture-backed drain is pending OR platformHint.javascript.backsideHooksInterval is running. That disjunction is right for media, since browsers relax autoplay once the page has engagement, but not for a popup: the interval drains from Window.setInterval, which grants no transient activation, so _blank queued the open and returned without a Sheet while the browser could block it silently -- the user gets nothing, which is worse than the Sheet _blank promises. Gate the popup path on a new isGestureBackedHookAvailable(), which reads only the semaphore raised by installBacksideHooksInUserInteraction() off a real pointer or key event. Interval-only availability now falls through to the confirmation Sheet. The download call sites keep using isBacksideHookAvailable() on purpose: their authorization story is different and the blob flow there is what the Initializr Generate path depends on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Cloudflare Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7f9610f85
ℹ️ 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".
RFC 3986 allows a scheme of one ALPHA, so x:payload is a legitimate deep link, but isExternalUrl() demanded two characters and classified it as a storage path -- the lookup then failed and it took the no-content popup branch, ignoring an explicit _self or auto target. The two character floor was there to keep a Windows drive letter from reading as a scheme. That cannot arise on this port: getFileSystemSeparator() is '/', so no storage path it hands us is drive qualified. Drop it. Checked across 27 inputs: x:payload, s:1 and a1+b-c.d:z classify external alongside the existing cases, while :noscheme, 1abc:x, /dir/a:b.txt and the bare storage paths still classify local. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isExternalUrl() is true for data: as well, since it does carry a scheme, so execute() depends on the data: branch staying ahead of the external one -- otherwise a data: URL would navigate instead of downloading. The dependency was correct but unstated, which makes it easy to break in a reorder. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three review nits. Use "file".equalsIgnoreCase(scheme) rather than lowercasing through the default locale, where a Turkish locale folds "FILE" to "fIle" and would classify a file: URL as external. Accept only the three documented javascript.execute.target values. The undocumented self / blank / new aliases were surprising configuration surface; an unrecognized value is now logged rather than silently treated as auto, so a typo is diagnosable. Nothing has shipped with the aliases, so there is no compatibility concern. Drop the furl alias. url is never reassigned in execute(), so it is already effectively final and capturable by the anonymous classes directly. The alias only created the url-versus-furl split the review flagged. 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: 4689ddb5b5
ℹ️ 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".
backsideHooksSemaphore is what isGestureBackedHookAvailable() reads to decide a popup may be attempted, and both drain-rescue paths added recently raised it by calling runBacksideHooksInTimeout(). Those drains carry no activation, so the port could believe a gesture was pending, attempt window.open(), and have it blocked -- exactly the outcome the gesture check exists to avoid, and with the blocked-popup indicator the auto path was written to prevent. Only interaction-scheduled drains raise the semaphore now. Self-scheduled ones still tag their generation and still count toward pendingGestureDrains, which is what makes them able to run a stranded hook at all; they simply no longer assert a gesture that does not exist. 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: d63cc66a45
ℹ️ 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 144 screenshots: 144 matched. |
…ion scope The keyboard adapter installs hooks on keydown, keypress and keyup, all through the generation-advancing overload, so a single keystroke advanced it three times and an execute() from a keyPressed handler was superseded by its own keypress or keyup. Mouse and touch releases already passed false; keyup and keypress now do too, leaving keydown to begin the interaction. __cn1_eval_on_main__ evaluates through indirect eval, so the generated scripts ran in global scope: "var cn1a" and "var cn1w" became properties of the embedding page's global object, clobbering anything of those names, and a page declaring a top-level let or const cn1w made the eval throw outright, breaking the default popup path. Every generated script is now wrapped in a function. Checked by running the emitted scripts under node: all four parse, none leaks a global, and against a page that declares "let cn1w" the old form throws "Identifier 'cn1w' has already been declared" while the wrapped form runs. 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: 31f75bbc2b
ℹ️ 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
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
Ports/JavaScriptPort/src/main/java/com/codename1/impl/html5/HTML5Implementation.java:7774
addBacksideHook(confirmed)inside the confirmation-sheet OK path can still end up “stranded” (never drained) on Safari/slow-EDT scenarios—this PR already introducesaddBacksideHookEnsuringDrain()specifically to prevent that for user-confirmed actions (see its javadoc and howdownloadDataUrl()uses it). Recommendation: useaddBacksideHookEnsuringDrain(confirmed)here whenhandlerRegisteredis true so a confirmed download reliably runs (or at least attempts) even if no drain remains.
if (handlerRegistered) {
addBacksideHook(confirmed);
} else {
addGestureOnlyHook(confirmed, new Runnable() {
@Override
public void run() {
showOpenConfirmation(url, "Open this link?");
}
});
}
docs/developer-guide/Working-With-Javascript.asciidoc:628
- “It always works” is stronger than what the implementation guarantees in all environments (e.g., the code has an explicit “web runtime out of date / main-thread bridge unavailable” path where links can’t be opened). Suggest softening this to “is not subject to the popup-blocker user-activation requirement” (or similar) so the statement stays accurate even under the documented compatibility fallback conditions.
Opening a link deserves its own note, because the restriction above bites almost every app that calls `Display.execute()`. Codename One dispatches events on its own EDT, so by the time your action listener runs, the browser no longer sees the tap as an ongoing user gesture and refuses to open a new window. Navigating the page the app already runs in has no such restriction: it always works, at the cost of unloading the app.
…authorities A slashless special-scheme URL is only an authority form when its scheme differs from the page's. When it matches, it is a relative reference: on an https page https:example.com/p resolves to the PAGE's host with example.com beginning the path. The Sheet named example.com as the destination, which is simply false -- and in the direction that matters, since it also meant https:trusted@evil.example/p was displayed as evil.example when the browser would never go there. Compare against the page's scheme, which the port already reads through mainLocationPart(), and show the page's own host for the relative case. Cross-scheme slashless forms keep their authority parsing, so http:trusted@evil.example/path still displays evil.example. Confirmed the resolution rules against node's URL parser first: https:example.com/path + base https://app.example.org/base/ -> https://app.example.org/base/example.com/path http:example.com/path + same base -> http://example.com/path 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: 49a35dae03
ℹ️ 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 separators after the colon, in any mix of "/" and "\", are an authority marker even when the scheme matches the page's, so https:\\evil.example/path navigates to evil.example from an https page. The same-scheme check I added last commit ran first and returned the app's own host, naming a destination the browser was not going to -- the dangerous direction, and introduced by the fix for the harmless one. Check for the marker before applying the relative rule. Verified against node's URL parser as the oracle rather than against my own expectations: for eleven shapes, the host shown now equals the host the parser resolves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
Ports/JavaScriptPort/src/main/java/com/codename1/impl/html5/HTML5Implementation.java:679
- The
8-generation cutoff is a behavior-defining magic number. Please extract it into a named constant (e.g.,MAX_GESTURE_HOOK_GENERATION_AGE) so it’s easier to audit/tune and so related logic stays consistent if this threshold ever changes.
if (gestureGeneration - stale.generation > 8) {
Ports/JavaScriptPort/src/main/java/com/codename1/impl/html5/HTML5Implementation.java:7049
showCannotOpenMessage()largely duplicates the layout logic thatcreateConfirmationSheet()provides (message + optional detail + centered buttons). Consider reusingcreateConfirmationSheet()(or extending it to support a single-button variant) so the sheet layout/styling remains consistent and future changes don’t need to be applied in two places.
private void showCannotOpenMessage(String url) {
final Sheet sheet = new Sheet(null, "Open Link");
SpanLabel message = new SpanLabel(
"This app cannot open links. Its web runtime is out of date.");
Button close = new Button("Close");
close.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent evt) {
sheet.back();
}
});
sheet.getContentPane().setLayout(BoxLayout.y());
sheet.getContentPane().add(message);
Label detail = new Label(shortenUrlForDisplay(url));
detail.setEndsWith3Points(true);
sheet.getContentPane().add(detail);
sheet.getContentPane().add(FlowLayout.encloseCenter(close));
sheet.show();
}
After the ninth review round on URL display, I stopped fixing shapes one at a time and diffed the whole thing against node's URL parser instead. That found 42 divergences nobody had reported yet, in two classes. A string with no scheme resolves against the page, so its destination host is the page's own and the text is a path beneath it. Showing the text alone invited reading "evil.example" as the destination when the link actually goes to <page host>/evil.example. Those now render as the page host plus the path, or the host and an ellipsis when too long. file: is WHATWG-special for parsing even though this port routes it to storage, so file:\\host/p carries a real authority. isSpecialScheme() now says so; routing is unaffected, since isExternalUrl() decides that separately. Consolidating the scheme grammar into schemeColon() removed the last four: a colon inside a path, as in /host:8443/p or ./user:pw@host, was being read as a scheme terminator. Both isExternalUrl() and the display parser now share that one definition rather than carrying a copy each. The corpus covers 13 schemes x 9 separator spellings x 6 authority shapes x 4 tails, plus whitespace and degenerate forms. For all 1689 inputs with a resolvable host, the host shown now equals the host node resolves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
Differential test against a real URL parserAfter the ninth review round on URL display, I stopped fixing shapes one at a time and diffed That found 42 divergences nobody had reported yet, in two classes:
Consolidating the scheme grammar into one Result: 0 divergences across all 1689 corpus inputs that have a resolvable host — including every shape found in review ( Two things worth saying plainly:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f182f3237
ℹ️ 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".
Clicking the anchor ourselves avoided the bridge's duplicate download and restored the retry, but it escaped the whole base64 payload into a JavaScript literal: several payload-sized copies on the worker, then a page-side parse of a multi-megabyte string as source. That is a worse cost than either problem it solved, and on the same payloads the storage gate already refuses to copy. Three constraints, and the current bridge can satisfy two: download once, retry when blocked, do not copy the payload. Registering fires eagerly, so re-firing duplicates; clicking ourselves costs the copies. Register and return, then -- one download, and the payload travels once as structured host-call data. The retry is what is given up. Recovering all three needs __cn1_register_save_blob_dataurl__ to accept a no-eager-fire flag, which lives in the translator repo rather than here. Drops downloadDataUrlOnMainThread() and addBacksideHookEnsuringDrain(), the latter having existed only to schedule a drain for that click. Net 63 lines lighter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
docs/developer-guide/Working-With-Javascript.asciidoc:644
- This table’s cell markers appear to be incorrect Asciidoctor syntax: using
||generally escapes a leading|rather than starting a new cell, and||Value |Behaviorcan render with an unintended literal|or misaligned columns. Recommend switching to standard table syntax (single|per cell) such as|===then|Value |Behavior, and for each row|-delimited cells (either on one line per row or one cell per line), so the rendered table reliably has exactly 2 columns.
[cols="1,4"]
|===
|Value |Behavior
|`auto`
|The default. Opens a new tab while the page still has user activation, and navigates the current page when it doesn't. No confirmation sheet is ever shown.
|`_blank`
|Only ever opens a new tab. When the browser blocks that, the port shows a confirmation sheet whose OK button supplies the gesture the browser wanted. This was the behavior before the property existed.
|`_self`
|Always navigates the page the app runs in.
|===
CodenameOne/src/com/codename1/ui/CN.java:670
- The reference
See [Display#execute(String)]is unlikely to resolve in Javadoc tooling as written (it looks like Markdown link text without a URL). Prefer a Javadoc link tag (e.g.,{@link Display#execute(String)}) or whatever link convention is used elsewhere in these///docs so this cross-reference renders as an actual clickable API link.
/// On the JavaScript port this can open a new tab, navigate the current page
/// or show a confirmation `Sheet`, depending on the
/// `javascript.execute.target` property. See [Display#execute(String)] for
/// the details.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
docs/developer-guide/Working-With-Javascript.asciidoc:644
- The table delimiters/cell markers look incorrect for Asciidoctor tables (
||===,||Value |Behavior, etc.). This may render as an extra empty column or malformed table. Use the standard table delimiter|===and single-cell markers (|Value |Behavior,|auto, etc.) so the table structure is unambiguous.
[cols="1,4"]
|===
|Value |Behavior
|`auto`
|The default. Opens a new tab while the page still has user activation, and navigates the current page when it doesn't. No confirmation sheet is ever shown.
|`_blank`
|Only ever opens a new tab. When the browser blocks that, the port shows a confirmation sheet whose OK button supplies the gesture the browser wanted. This was the behavior before the property existed.
|`_self`
|Always navigates the page the app runs in.
|===
docs/developer-guide/Working-With-Javascript.asciidoc:637
- The statement “No confirmation sheet is ever shown” is too absolute given the implementation has explicit compatibility/failure fallback paths that can still show a Sheet (e.g., when main-thread eval fails and the code degrades). Consider softening to “is not shown in the normal path” (or explicitly scoping the claim to “when the main-thread bridge is available and the page-side script succeeds”).
|`auto`
|The default. Opens a new tab while the page still has user activation, and navigates the current page when it doesn't. No confirmation sheet is ever shown.
CodenameOne/src/com/codename1/ui/CN.java:670
See [Display#execute(String)]reads like a Markdown link but doesn’t include a URL/target, so it may render literally instead of linking (depending on the doc toolchain for these///comments). Use the project’s established intra-doc link format (e.g.,Display#execute(String)as plain text, or a proper Javadoc/link directive if supported) to ensure the cross-reference renders correctly.
/// On the JavaScript port this can open a new tab, navigate the current page
/// or show a confirmation `Sheet`, depending on the
/// `javascript.execute.target` property. See [Display#execute(String)] for
/// the details.
CodenameOne/src/com/codename1/ui/Display.java:4096
- Similar to the developer guide, “No confirmation prompt is ever shown” is stronger than what the implementation can guarantee in failure/compatibility scenarios (e.g., missing/blocked main-thread bridge or eval failures causing fallback UI). Recommend qualifying the statement (e.g., “in the normal path”) or describing the documented degradation behavior.
/// - `auto` (the default) opens a new tab when the page still has user
/// activation and otherwise navigates the page the app is running in. No
/// confirmation prompt is ever shown, but note that navigating the current
/// page unloads the app.
Ports/JavaScriptPort/src/main/java/com/codename1/impl/html5/HTML5Implementation.java:6989
- Logging only
t.toString()drops the stack trace, which will make diagnosing bridge/script issues much harder (especially since this method intentionally uses exceptions for control flow like “popup blocked”). Prefer logging the throwable with stack trace using the project’s standard logging mechanism (e.g., a logger overload that acceptsThrowable, or a dedicatedLog.e(t)-style call) while keeping the user-facing behavior unchanged.
} catch (Throwable t) {
_log("Failed to open URL on the main thread: " + t);
return false;
}
Collaborator
Author
|
Compared 181 screenshots: 181 matched. |
Collaborator
Author
|
Compared 148 screenshots: 148 matched. Benchmark Results
Detailed Performance Metrics
|
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
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.
Problem
Opening a link from the JavaScript port went through a confirmation
Sheetwhenever the browser no longer saw a live user gesture — which is almost always, since Codename One dispatches events on its own EDT, so by the time an action listener callsDisplay.execute()the browser has stopped counting the tap.Three things were wrong:
EASTof aSpanLabelholding the full URL. A URL has no spaces forSpanLabelto wrap on, so the label's preferred width blew past the screen and carried the buttons off-edge, where they could not be tapped.CN.execute(furl), which re-enteredexecute()with still no gesture registered and could show the sheet again indefinitely.window.open(...)from inside the Web Worker the app runs in, wherewindow.opendoes not exist — so even the backside-hook path threw.Fix
A new
javascript.execute.targetproperty:autonavigator.userActivation.isActive), and navigates the current page otherwise. No sheet is ever shown, and no blocked-popup indicator either._blank_selfURL opening now routes through
eval_(), whichport.jsalready forwards to the__cn1_eval_on_main__host bridge (the same channelDisplay.execute("javascript:...")uses). No new host-bridge handler is needed, so this does not require a matching translator change, and a page built against an olderbrowser_bridge.jsdegrades to the sheet instead of failing. The now-deadwindowOpennative is removed.Both sheets share a new
createConfirmationSheet()that stacks message / shortened URL / buttons inBoxLayout.y(), so nothing can push the buttons out of reach. The URL is shortened (scheme stripped, host kept, ellipsized) into aLabelwithsetEndsWith3Points(true). OK now opens the URL directly rather than recursing.Docs
Display.executegets a "JavaScript port" javadoc section explaining why the gesture is lost and listing the three target values;CN.executecross-references it. The developer guide's Playing media and opening links section gains a Choosing where a link opens subsection with a value table and a snippet fixture — and its opening paragraph is corrected, since it still claimed links always prompt.Behavior change to be aware of
Under
autoand_selfthe app is unloaded whenever the current page navigates away, losing in-memory state. This is documented in both the javadoc and the guide, with_blanknamed as the escape hatch for apps that need to stay alive.Verification
JS port sources and both snippet fixtures compile;
validate-guide-snippets.py,check-copyright-headers.sh, Vale, asciidoctor, paragraph-capitalization and LanguageTool (0 matches) all pass locally. Not exercised in a real browser — theautopath's behavior against a live popup blocker is reasoned from the API contract rather than observed.🤖 Generated with Claude Code