Slow DLP dialog does not show correctly when action done in chat sidebar
Categories
(Firefox :: Data Loss Prevention, defect)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox-esr140 | --- | unaffected |
| firefox-esr153 | --- | affected |
| firefox153 | --- | wontfix |
| firefox154 | --- | affected |
| firefox155 | --- | affected |
People
(Reporter: gstoll, Assigned: gstoll)
References
(Regression)
Details
(Keywords: regression)
Attachments
(2 files)
STR:
- Run content_analysis_sdk_agent.exe with a delay of a few seconds to ensure the slow dialog will show up
- Turn on content analysis in Firefox (set browser.contentanalysis.enabled=true, then restart Firefox with the command line argument "--allow-content-analysis")
- Open the AI chat sidebar and paste some text into it
The slow DLP dialog will show up as window modal, which is wrong and also means that we can't automatically dismiss it when the DLP action is done.
| Assignee | ||
Comment 1•2 days ago
|
||
mozregression points to bug 1980404 - almost certainly the change to isEmbeddedInSidebar() caused this, as we use that in openPromptWithTabDialogBox()
Confirming comment 1: the regressing change is the browser/actors/PromptParent.sys.mjs hunk of D297073 (bug 1980404), which added chatbot-browser to the messagemanagergroup exclusion list in isEmbeddedInSidebar(). Before bug 1980404 the chatbot <browser> had no messagemanagergroup, so isEmbeddedInSidebar() returned true and openPromptWithTabDialogBox() hoisted to <browser id="sidebar"> (bug 1955250). Now it returns false, win becomes the chat.html window which has no gBrowser, and the prompt falls through to the window-modal Services.ww.openWindow() branch. That also explains the second symptom: ContentAnalysis._disconnectFromView() computes the sidebar hoist without that exclusion, so it looks for a tab dialog that was never created and can't dismiss it.
That exclusion only exists for extension sidebar browsers (bug 1513656); the chatbot browser is nested exactly like the case bug 1955250 handles, so the fix is to drop it from the list: https://phabricator.services.mozilla.com/D317584
Caveats: I could not verify this at runtime (no Windows / content_analysis_sdk_agent here). The patch adds a browser test in browser_chat_sidebar.js that exercises the regression without a DLP agent, but it has not been run - it needs a try push, and the sidebar tab-dialog path had no test coverage before, so watch for leaks.
The prefs in the STR are a supported configuration, so no unsupported-config: browser.contentanalysis.enabled is set by enterprise policy in production, and --allow-content-analysis is only needed when the pref is flipped manually instead.
If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
Bug 1980404 added messagemanagergroup="chatbot-browser" to the AI chatbot
sidebar <browser>, and also added that group to the exclusion list in
PromptParent.isEmbeddedInSidebar().
Before that change the chatbot browser had no messagemanagergroup
attribute, so isEmbeddedInSidebar() returned true and
openPromptWithTabDialogBox() hoisted the browser to <browser id="sidebar">,
which is what bug 1955250 added so that Content Analysis can show tab-modal
dialogs in the sidebar. With the exclusion it returns false, win ends up
being the chat.html window (which has no gBrowser), and the prompt falls
through to the window-modal Services.ww.openWindow() branch. That also
breaks ContentAnalysis._disconnectFromView(), which still hoists to the
sidebar browser and therefore cannot find the dialog to abort when the DLP
request finishes.
The exclusion is only there for extension sidebar browsers, which have more
layers of nesting and leak windows in tests (bug 1513656). The chatbot
browser is nested exactly like the case bug 1955250 handles, so drop it from
the list.
Adds a regression test that requests a tab-modal prompt for the chatbot
browser and checks it is opened as a tab dialog on the sidebar browser and
can be closed programmatically, which does not need a DLP agent.
Comment 4•2 days ago
|
||
Set release status flags based on info from the regressing bug 1980404
:mkaply, since you are the author of the regressor, bug 1980404, could you take a look? Also, could you set the severity field?
For more information, please visit BugBot documentation.
| Assignee | ||
Comment 5•2 days ago
|
||
I’m already looking at this, thanks though bugbot 🙂
Updated•1 day ago
|
| Assignee | ||
Comment 6•1 day ago
|
||
Separate the concepts of whether something is embedded in the sidebar versus whether
tab dialog boxes should get shown there. Also add a test as this is historically a fragile
area.
Updated•1 day ago
|
Description
•