Repository navigation
Replace webview based tags control - #437
tonygermano merged 2 commits into
Conversation
ssrowe
added
the
licensing
|
Tagging this This PR deletes JS/CSS that was committed straight into Replacing these with Swing removes all of them from the IP review list. |
There was a problem hiding this comment.
MirthTagField.java:383: Tags that scroll off the left edge can’t be reached again. The tag row never shows a horizontal scrollbar, and the only scrolling code moves the text box into view. Once the tags are wider than the field, the first ones disappear past the left edge. Mouse-wheel scrolling doesn’t work without a visible scrollbar, and the old Home/arrow-key navigation is gone. The channel editor has no clear button, so removing the first tag means backspacing through every tag after it. It happens with roughly 8–10 long tag names. The Dashboard and Channels filter fields (max 500px wide) can hit it too. Old field overflowed too, but had keyboard navigation enabled.
Otherwise LGTM.
The tag field is a JavaFX WebView rendering a bundled jQuery and bootstrap-tokenfield page. It was injected with WebEngine.loadContent, which gives the document an about:blank origin, and since the July 2026 OpenJFX critical patch update WebKit refuses the page's jar:-scheme scripts and stylesheets from that origin. The field then renders as a bare text input: text can be typed but never becomes a tag, in the channel list, the channel editor and the dashboard. Load the page by its own resource URL instead, so the document shares an origin with its resources. The loadContent approach was a workaround for JDK-8136529, a Java 8u60-8u71 Web Start bug, and Web Start was removed in Java 11. This is not a regression in any BridgeLink release. The code is identical in v26.6.0 and dates to 2016; what changed is the JavaFX bundled with the JRE that runs the Administrator. The server's Java is not involved. Verified on macOS arm64 against Zulu FX 21.0.12.1 (fails before, passes after), 21.0.11 and 17.0.10 (pass either way), and end to end in the Administrator against a local 26.9.0 server. Also in this change: - Log the load worker's FAILED state, and wrap the JS bridge calls in the SUCCEEDED branch. Both failure modes were silent before, which is why this went unnoticed from the July 2026 update until a community report. - Clear ChannelPanel.isEditingChannel in a finally block. Three early returns and any thrown exception skipped the reset and left it latched on, which disables double-click editing for the rest of the session. This is a latent defect found while investigating, not the cause of the reported symptoms: double-click editing and New Channel were both verified working on a failing runtime with the unfixed tag field. - Add client/manual-checks/tagfield, a manual check that drives the real MirthTagWebBrowser. It cannot run in CI, which has a jdk+fx JDK but no display, and it lives outside client/test because *Test.class there is collected automatically. Ported from BridgeLink (github.com/Innovar-Healthcare/BridgeLink) commit 322342082d553351b6701b89f4321d41535218e7, released in v26.9.0; MPL-2.0. Applied unchanged except: the .gitignore hunk re-anchored to our file, SPDX headers on the new manual-check files naming Innovar Healthcare, and the README pointing at our .github/workflows/build.yaml. OIE upstream tracks the bug as OpenIntegrationEngine#434; its fix there (OpenIntegrationEngine#437, Swing tag field) is unmerged.
71906cf to
9f7afb7
Compare
tonygermano
left a comment
tonygermano
left a comment
There was a problem hiding this comment.
Thanks for this. Dropping the WebView, the FX thread hops and the bundled JS/CSS is a clear win, and the Swing version is much easier to follow than the old Java ↔ JS round trips.
Two things I'll handle in a follow-up PR after this merges. I already have the commits for both on a branch built on top of this one:
-
Tags that scroll off the left edge can't be reached. +1 to @NicoPiel's comment. With
HORIZONTAL_SCROLLBAR_NEVER,BasicScrollPaneUIignores the mouse wheel, and the only scroll call keeps the editor in view. In the channel editor (no clear button), the only way to remove the first tag is to backspace through every tag after it. The follow-up adds:- Scrolling the chips with the mouse wheel or a sideways trackpad swipe.
- Separate tag navigation and outlined edit modes, which decide what the arrow keys, Home, End, Backspace and Delete do.
- Middle-click to delete a chip, like a browser tab, and a left click in the gap between chips to place the editor there.
-
Orphaned third-party notice. This PR removes the last copy of bootstrap-tokenfield, so its section in
server/docs/thirdparty/THIRD-PARTY-README.txt(lines 412–441, "The license below pertains to the bootstrap-tokenfield library") should be removed too. It's in the same follow-up. The jQuery and Bootstrap sections need to stay, sinceserver/public_api_htmlstill shipsjquery-3.5.1.min.jsandbootstrap-glyph.css. jquery-ui never had a notice, so there's nothing to remove for it.
One non-blocking note: in the filter fields, isDuplicate still ignores the type, so a tag and a channel with the same name can't both be added. This is unchanged from before and not something this PR needs to fix. I'm only mentioning it because the second commit is about that same name collision.
What checks out
Claude compared the new MirthTagField against the old HTML/JS behavior and checked the callers in ChannelSetup, ChannelPanel and DashboardPanel.
- The
MirthFieldConstraints(MAX_TAG_LENGTH, false, true, true)document allows the same characters as the oldcreatetokenregex ([a-zA-Z0-9\s\-_]), and the 24-character limit still applies only in the channel editor. The 10-tag limit and the duplicate rules (case-insensitive in the channel editor, exact match elsewhere) are unchanged. - Enter, comma, Down and Escape behave as before, including Enter taking the highlighted completion when the popup is open.
createTagOnFocusLost()is now synchronous, so a tag typed and not yet committed is ingetTags()beforesaveChannelTags()reads it. The old path went throughPlatform.runLaterand JS, so it could race the save.- Search listeners now run on the EDT. The old path called them from the FX thread (for example
parent.setSaveEnabled(true)inChannelSetup). - Looking up colors by
type:namewith a case-insensitive key fixes the channel/tag name collision. It also means typingFOOin the channel editor shows the existingfootag's color, which matches howsaveChannelTags()links them withequalsIgnoreCase. - The branch still merges cleanly with current
main, and CI is green.
Claude's checks were done by reading the code.
Drafted with Claude Code
The tag field ran bootstrap-tokenfield and jQuery in a JavaFX WebView, so every change went back and forth between the EDT, the FX thread and JavaScript. At idle the WebView kept the client at about 2% CPU and used about 100 MB more memory than the client does without it. MirthTagField now lays out TagChip components and a text field in a row that scrolls horizontally, and shows RSyntaxTextArea's completion popup itself. MirthTagWebBrowser, MirthTagWebController, AutoCompletionPopupWindow and the bundled HTML, CSS and JavaScript are removed, along with the clientJar rule that packaged them. Because everything now happens on the EDT, search listeners are called there, and createTagOnFocusLost() commits typed text before ChannelSetup reads the tags, instead of racing it through the FX thread. Backspace in an empty field deletes the last tag at once; bootstrap-tokenfield selected it on the first press. Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Commit-message-by: Tony Germano <tony@germano.name> Signed-off-by: Tony Germano <tony@germano.name>
Chip colors were looked up by name alone, so a channel and a tag with the same name shared one entry and whichever was added last won. The Dashboard and Channels panels build their completions in a HashSet of objects with identity hash codes, so the order changed from one refresh to the next, and the tag's chip switched between its own color and the plain channel-name color. Look colors up by type and name. In the channel editor everything typed is a tag, so a name token is looked up as a tag there, case insensitively, as before. Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Commit-message-by: Tony Germano <tony@germano.name> Signed-off-by: Tony Germano <tony@germano.name>
9f7afb7 to
6248cdd
Compare
The tag field is a JavaFX WebView rendering a bundled jQuery and bootstrap-tokenfield page. It was injected with WebEngine.loadContent, which gives the document an about:blank origin, and since the July 2026 OpenJFX critical patch update WebKit refuses the page's jar:-scheme scripts and stylesheets from that origin. The field then renders as a bare text input: text can be typed but never becomes a tag, in the channel list, the channel editor and the dashboard. Load the page by its own resource URL instead, so the document shares an origin with its resources. The loadContent approach was a workaround for JDK-8136529, a Java 8u60-8u71 Web Start bug, and Web Start was removed in Java 11. This is not a regression in any BridgeLink release. The code is identical in v26.6.0 and dates to 2016; what changed is the JavaFX bundled with the JRE that runs the Administrator. The server's Java is not involved. Verified on macOS arm64 against Zulu FX 21.0.12.1 (fails before, passes after), 21.0.11 and 17.0.10 (pass either way), and end to end in the Administrator against a local 26.9.0 server. Also in this change: - Log the load worker's FAILED state, and wrap the JS bridge calls in the SUCCEEDED branch. Both failure modes were silent before, which is why this went unnoticed from the July 2026 update until a community report. - Clear ChannelPanel.isEditingChannel in a finally block. Three early returns and any thrown exception skipped the reset and left it latched on, which disables double-click editing for the rest of the session. This is a latent defect found while investigating, not the cause of the reported symptoms: double-click editing and New Channel were both verified working on a failing runtime with the unfixed tag field. - Add client/manual-checks/tagfield, a manual check that drives the real MirthTagWebBrowser. It cannot run in CI, which has a jdk+fx JDK but no display, and it lives outside client/test because *Test.class there is collected automatically. Ported from BridgeLink (github.com/Innovar-Healthcare/BridgeLink) commit 322342082d553351b6701b89f4321d41535218e7, released in v26.9.0; MPL-2.0. Applied unchanged except: the .gitignore hunk re-anchored to our file, SPDX headers on the new manual-check files naming Innovar Healthcare, and the README pointing at our .github/workflows/build.yaml. OIE upstream tracks the bug as OpenIntegrationEngine#434; its fix there (OpenIntegrationEngine#437, Swing tag field) is unmerged. [4.7.0 port] The server/mirth-build.xml hunk is dropped (Ant is a tombstone upstream); the manual check is pointed at the Gradle build output in a follow-up commit of this port. (cherry picked from commit 36e4816)
Reduces the JavaFX based WebView + JQuery + Swing tags control to only use swing. I think the prior webview design was to "test the waters" for using WebView and JFX, not as an actual requirement.
Performance impact is good. Drops idle CPU usage from ~2% to ~0.2%, drops memory usage by ~100 MB. Improves MacOS "power score" for the application by 88% - so there should be some benefit to battery life.
Also includes a drive-by fix for a bug where tag colors would oscillate on refresh if you had a channel and tag named the same thing.
Not a direct fix, but would close #434