Skip to content

fix(frontend): restore network topology rendering in Safari - #1834

Merged
jokob-sk merged 2 commits into
netalertx:mainfrom
itwormz:fix/safari-network-topology
Oct 4, 2026
Merged

jokob-sk merged 2 commits into
netalertx:mainfrom
itwormz:fix/safari-network-topology

Conversation

@itwormz

@itwormz itwormz commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

📌 Description

Safari renders the network topology's SVG links, but paints device cards,
labels and icons at the SVG origin. The cards overlap in the upper-left
corner instead of appearing at the ends of their connections.

The nodes use HTML inside SVG foreignObject. Positioned elements and
opacity layers trigger WebKit's rendering issue. This change keeps their
layout in normal flow and uses alpha colors for muted icons:

  • Override AdminLTE's inherited relative positioning only inside the network tree.
  • Keep labels and hardware icons in normal flow.
  • Place collapse controls with a float and a negative margin derived from node height.
  • Replace icon opacity with an alpha color, with a visible fallback for browsers without color-mix.

The hierarchy, links, device data and click handlers retain their existing contracts.

Commit structure

The implementation and regression coverage are deliberately separate:

  1. 071a4dd — Safari fix only: network-tree CSS and the collapse control's height-derived margin in front/js/network-tree.js.
  2. 1aceeae — Regression tests and documentation only: the standalone browser test, HTML fixture and testing guide.

The fix can be reviewed or cherry-picked independently. The test commit introduces
no production dependency; Playwright and pngjs are installed separately for test execution.

🔍 Related Issues

📋 Type of Change

  • 🐛 Bug fix
  • 🧪 Test addition or change

📷 Screenshots or Logs

The regression fails on the original code with:

#networkTree .spanNetworkTree is not painted at its SVG coordinates

The same test passes after the fix in WebKit, Chromium and Firefox.
DOM bounds alone are insufficient: WebKit can report correct bounds while
painting the HTML at the wrong position, so the test checks screenshot pixels too.

🧪 Testing Steps

  • Reproduced the original visual defect and verified the corrected fixture in native macOS Safari.
  • Automated WebKit, Chromium and Firefox checks: device labels and icons, node clicks, collapse/expand, wheel zoom and drag pan. The test asserts that zoom and pan actually change the SVG transform.
  • Verified the regression fails on the original CSS/renderer and passes with the fix.
  • JavaScript syntax checks and git diff --check pass.
  • Ran the full pytest test/ suite on Debian. The non-Docker tests passed: 1665 passed, 30 skipped.
  • Re-ran all Docker tests as root with a daemon-visible temporary directory: 75 passed, 2 failed. This resolved checkout permissions and nested bind-mount issues in the initial run.
  • Both remaining failures also reproduce with the original CSS and renderer from 48fa0392: test_host_network_compose and test_normal_startup_no_warnings_compose. The existing host-network check treats the Debian LXC host's eth0@if… interface as Docker bridge networking and emits an unexpected warning even with network_mode: host.

The standalone browser regression and its installation commands are documented
in test/ui/TESTING_GUIDE.md. It runs separately from pytest and needs no
NetAlertX backend or database.

✅ Checklist

  • I have read the Contribution Guidelines.
  • I have tested the change in native Safari and three automated browser engines.
  • I have documented how to run the regression.
  • I have checked existing network interactions in Chromium and Firefox.
  • Full pytest suite passes (two existing Compose failures reproduce on the unmodified frontend in this LXC environment).

🙋 Additional Notes

AI-assisted contribution: The implementation, regression tests and PR description were prepared with OpenAI Codex. Human-reviewed and approved for submission by @itwormz.

Native iPhone/iPad Safari has not been tested. This PR fixes the observed
network topology rendering issue; it does not declare general Safari support.

Summary by CodeRabbit

  • Bug Fixes
    • Improved the positioning of network-tree boxes, labels, and collapse controls.
    • Updated port and hardware icon coloring to render more consistently, including within the network tree.
  • Tests
    • Added browser-based checks for topology rendering and interactions, including clicks, collapse and expand, zoom, and pan across supported browsers.
  • Documentation
    • Added instructions for running the standalone topology browser checks, including where test artifacts are saved.

WebKit paints positioned HTML and opacity layers inside SVG foreignObject
at the SVG origin. Device cards, labels and icons overlap there while
their connections remain at the expected coordinates.

Keep node content in normal flow, place collapse controls with a float
and height-derived negative margin, and use alpha colors for muted icons.
Scope all style changes to the network tree.

Related: netalertx#1116
Discussion: netalertx#1379
Add a standalone fixture using the real topology renderer and styles.
Check painted pixels in WebKit, Chromium and Firefox because DOM bounds
can remain correct when WebKit paints foreignObject content incorrectly.

Exercise node clicks, collapse/expand, wheel zoom and drag pan, including
assertions that pointer gestures change the SVG transform.

Document dependency installation and execution separately from pytest.
The regression fails with the original frontend and passes with the fix.
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
CLAUDE.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: netalertx/NetAlertX/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: cfc901f0-89f7-42d2-a578-f0836345efd9
📥 Commits

Reviewing files that changed from the base of the PR and between 48fa039 and 1aceeae.

📒 Files selected for processing (5)
  • front/css/app.css
  • front/js/network-tree.js
  • test/ui/TESTING_GUIDE.md
  • test/ui/fixtures/network_topology.html
  • test/ui/test_network_topology.cjs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Network-tree positioning and icon styling changed. The collapse control uses a height-based negative margin. A standalone regression test checks rendering and interactions in WebKit, Chromium, and Firefox.

Changes

Network topology

Layer / File(s) Summary
Network-tree rendering adjustments
front/css/app.css, front/js/network-tree.js
Network-tree boxes, text, collapse controls, and hardware icons now use static positioning. Icon color declarations replace opacity for reduced intensity. The collapse control uses a negative margin based on node height.
Cross-browser topology regression
test/ui/fixtures/network_topology.html, test/ui/test_network_topology.cjs, test/ui/TESTING_GUIDE.md
A fixture and standalone test check rendered pixels, node clicks, collapse and expand, wheel zoom, and drag pan in WebKit, Chromium, and Firefox. The guide documents setup and execution.

Suggested reviewers: jokob-sk

Priority: ➖ Normal

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 1acee

No confirmed issue currently blocks merging the topology fix after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1acee

The rendering fix has limited architectural impact. The new test server is restricted to loopback, but its file-serving check can follow repository symlinks outside the repository. Exposure is conditional and limited to the test host; no production endpoint or actual data disclosure was demonstrated.

Retained concerns

  • Low · security · inferred: The new unauthenticated loopback server can serve files outside its intended repository root through an in-root symlink. A client able to reach the test host's loopback listener could request files through api, which points to /tmp/api, if that target exists and is readable by the runner. The inspected target is absent, so disclosure is conditional rather than demonstrated.
Security review details

Security Blast Radius

  • inferred — Direct exposure is limited to clients able to reach the test host's loopback listener while the runner is active. File-read authority is inherited from the runner process and includes repository files and readable targets reached through repository symlinks; it is not limited to the fixture's required assets.

Security Findings and Attack Paths

  • inferred — A request beneath /api/ passes the lexical repository-prefix check, after which fs.readFile can follow the tracked api symlink to /tmp/api. This is a newly exposed conditional read path, not evidence that sensitive files exist there or were disclosed.

Trust Boundaries and Controls

  • observed — The handler resolves request paths beneath the repository and rejects paths outside the lexical root-plus-separator prefix. This controls ordinary traversal and prefix confusion but does not check canonical symlink targets. The listener is explicitly loopback-only.

Resilience and Maintainability Implications

  • observed — After successful startup, ordinary browser and assertion failures reach server cleanup in main's finally block. Successfully launched browsers have independent finally-based cleanup. Each run receives an ephemeral port, avoiding fixed-port collisions.

Hardening Proposals

  • proposed — Restrict serving to the fixture and required static assets, with filesystem containment that rejects outside-root symlink targets rather than relying only on lexical path validation.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Safari network-topology rendering fix, which is the primary change in the pull request.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (3 skipped: 3 …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@itwormz itwormz changed the title Fix/safari network topology fix(frontend): restore network topology rendering in Safari Oct 4, 2026
@itwormz
itwormz marked this pull request as ready for review October 4, 2026 15:09
@jokob-sk

jokob-sk commented Oct 4, 2026

Copy link
Copy Markdown
Collaborator

Thanks so much @itwormz 🙏 I thought I will never get this one fixed - I'm not into apple stuff 😄

@jokob-sk
jokob-sk merged commit 0dba888 into netalertx:main Oct 4, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants