fix: replace window.confirm with custom modal in embedded Plugin Page#210
Closed
YumemiDream wants to merge 2 commits into
Closed
fix: replace window.confirm with custom modal in embedded Plugin Page#210YumemiDream wants to merge 2 commits into
YumemiDream wants to merge 2 commits into
Conversation
The embedded Plugin Page runs inside a sandboxed iframe without 'allow-modals', so window.confirm() calls are silently blocked by the browser. This caused all batch operations (approve/reject/delete) on the reviews, jargon, and expression-learning pages to appear unresponsive. Replace all window.confirm() calls with a custom showConfirm() function that renders a confirmation dialog using the existing <dialog> element, which works within sandbox restrictions.
Contributor
Reviewer's GuideReplaces blocked window.confirm() calls in the sandboxed embedded Plugin Page with a custom Promise-based confirmation modal built on the existing element, ensuring batch review operations prompt correctly under iframe sandbox restrictions. Sequence diagram for batch action confirmation with showConfirmsequenceDiagram
actor User
participant Page as PluginPage
participant Handler as handleBatchReviewAction
participant Confirm as showConfirm
participant Modal as detail-modal
participant API as apiPost
User->>Page: click batch action button
Page->>Handler: handleBatchReviewAction(action, kind)
Handler->>Confirm: showConfirm(title, message, confirmText)
Confirm->>Modal: setText(modal-title, title)
Confirm->>Modal: setHtml(modal-body, message+buttons)
alt modal supports showModal
Confirm->>Modal: modal.showModal()
else fallback open attribute
Confirm->>Modal: modal.setAttribute(open, "")
end
User-->>Modal: click confirm-ok
Modal->>Confirm: modal.close()
Confirm-->>Handler: Promise resolved true
Handler->>API: apiPost(...)
API-->>Handler: result
Handler-->>User: batch operation completes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
showConfirm, each invocation adds a newcancellistener to the shareddetail-modaldialog that is only cleaned up ifcancelactually fires, so over time multiple unused listeners can accumulate; consider using a single persistent handler or explicitly removing the listener on any path that closes the dialog. - The fallback to
window.confirm(message)whendetail-modalis missing will still silently fail (returnfalse) in the sandboxed iframe environment; if this path is expected to occur there, you may want a different fallback behavior (e.g., always resolvingtrueor showing an inline warning) instead of inheriting the original bug.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `showConfirm`, each invocation adds a new `cancel` listener to the shared `detail-modal` dialog that is only cleaned up if `cancel` actually fires, so over time multiple unused listeners can accumulate; consider using a single persistent handler or explicitly removing the listener on any path that closes the dialog.
- The fallback to `window.confirm(message)` when `detail-modal` is missing will still silently fail (return `false`) in the sandboxed iframe environment; if this path is expected to occur there, you may want a different fallback behavior (e.g., always resolving `true` or showing an inline warning) instead of inheriting the original bug.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…nferred When the v2 learning system encounters a jargon term that already exists and is_complete=True (manually edited or fully inferred), skip the save to prevent overwriting the user's edits. Also preserve the existing count when updating an existing record, instead of resetting it to 1. Fixes: manually edited jargon meanings get overwritten by re-learning.
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.
Description:
The embedded Plugin Page runs inside a sandboxed iframe without
'allow-modals', so window.confirm() calls are silently blocked by the
browser. This caused all batch operations (approve/reject/delete) on
the reviews, jargon, and expression-learning pages to appear unresponsive.
Replace all window.confirm() calls with a custom showConfirm() function
that renders a confirmation dialog using the existing element,
which works within sandbox restrictions.
Summary
内嵌 Plugin Page 运行在
<iframe sandbox="...">中,没有allow-modals权限,导致window.confirm()被浏览器静默拦截(返回
false)。所有使用window.confirm()的批量操作(审查队列、黑话学习、表达方式学习的批量批准/拒绝/删除)点击后直接 return,表现为"点击没反应"。
用已有的
<dialog>元素实现showConfirm()替代原生confirm(),在 sandbox 环境下正常工作。Changes
showConfirm()函数,基于<dialog>元素实现自定义确认弹窗handleBatchReviewAction中的window.confirm(审查队列批量操作)handleJargonBatchAction中的window.confirm(黑话学习批量操作)handleStyleBatchAction中的window.confirm(表达方式学习批量操作)Related Issues
Type of Change
Checklist
Summary by Sourcery
Replace blocked native confirmation dialogs in the embedded dashboard with a custom modal-based confirmation flow for batch actions.
Bug Fixes:
Enhancements: