Skip to content

fix: preserve filament configuration navigation during WebView initialization - #1182

Merged
LiuLikeQian merged 1 commit into
Snapmaker:mainfrom
zhangzhend0ng:codex/fix-filament-config-navigation
Nov 13, 2026
Merged

LiuLikeQian merged 1 commit into
Snapmaker:mainfrom
zhangzhend0ng:codex/fix-filament-config-navigation

Conversation

@LuckZAE

@LuckZAE LuckZAE commented Nov 13, 2026

Copy link
Copy Markdown

No description provided.

@zhangzhend0ng

Copy link
Copy Markdown
Collaborator

VERDICT: APPROVE_WITH_COMMENTS

Findings

# Location Severity Issue & suggested fix Evidence Tier
— — — No findings. — —

Author claims

PR description is empty (fix: preserve filament configuration navigation during WebView initialization is the title only). No behavioral claims in the description to verify; section omitted.

Checklist coverage

  • snapmaker-orca/coding-standards §1 Language Baseline (C), §2 Format Standards (C) — compliant. The const wxString& name reference style change matches .clang-format PointerAlignment: Left (C:\Users\SNAPMA~1\AppData\Local\Temp\codex-review-zu__3w59\wt\.clang-format:214); Allman braces preserved; no new exceptions/Chinese/BOM issues.
  • wxWidgets 3.1.5 §8 wxWebView: WKWebView Migration and Edge (P) — compliant. Change is Edge-only (#ifdef __WIN32__ / WebViewEdge), no GCC/Clang build fix or wxWebViewIE regression introduced.
  • gui module checklist (P) — compliant. Timer/bridge callback stays on the wx event loop; no new user-visible strings; no blocking work in event handlers; no new settings keys.
  • cpp/memory/ownership / cpp/concurrency/thread-safety — no new allocations or cross-thread shared state; existing wxWeakRef<WebViewEdge> guards the async completion callback.

Action items

  • Provide a PR description with the behavioral claim (e.g. "initial Win32 WebView navigation is deferred until the Edge script-message bridge is registered").
  • Windows QA: confirm the filament configuration page (WebPresetDialog, file://.../web/preset_bind/24/index.html) no longer shows welcome content.
  • Windows QA: regression-check the other WebView::CreateWebView entry points (WebUserLoginDialog, WebGuideDialog, WebViewDialog, PrinterCloudAuthDialog, WebDownPluginDlg, Project, ModelMall, ReleaseNote, MarkdownTip, PrivacyUpdateDialog, StatusPanel) still navigate after creation.
  • Behavior-change note (R2): CreateWebView previously loaded url2 via Create(parent, …, url2, …); the diff inserts webView->LoadURL(url2) with no equivalent in the old flow. The real navigation is now deferred until MarkBridgeScriptReady flushes m_pendingURL. Direction is correct and self-consistent, but it must be covered by the QA above.
  • Note: moving m_bridgeScriptState = Waiting before wxWebViewEdge::AddScriptMessageHandler makes the existing "fail-open" comment actually effective (previously MarkBridgeScriptReady(false) early-returned while state was still Idle); on native handler failure the page now loads without the bridge and emits wxEVT_WEBVIEW_ERROR. Verify no dialog's OnError misbehaves under that (rare) failure.

Limitation: wxWidgets' wxWebViewEdge::Create/AddScriptMessageHandler implementations are not vendored in-tree, so their exact internal navigation path could not be inspected; the diff's correctness does not hinge on that detail.


AI 评审(advisory),由 dev-guidelines harness 流水线生成 · head d6ef5e33 · engine=codex+tree

@LiuLikeQian
LiuLikeQian merged commit 5dd073e into Snapmaker:main Nov 13, 2026
1 check 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.

3 participants