Open Bug 2062316 Opened 2 days ago Updated 1 day ago

Slow DLP dialog does not show correctly when action done in chat sidebar

Categories

(Firefox :: Data Loss Prevention, defect)

defect

Tracking

()

ASSIGNED
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:

  1. Run content_analysis_sdk_agent.exe with a delay of a few seconds to ensure the slow dialog will show up
  2. Turn on content analysis in Firefox (set browser.contentanalysis.enabled=true, then restart Firefox with the command line argument "--allow-content-analysis")
  3. 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.

mozregression points to bug 1980404 - almost certainly the change to isEmbeddedInSidebar() caused this, as we use that in openPromptWithTabDialogBox()

Keywords: regression
Regressed by: 1980404
See Also: → 1955250

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.

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.

I’m already looking at this, thanks though bugbot 🙂

Flags: needinfo?(mozilla)

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.

Assignee: nobody → gstoll
Status: NEW → ASSIGNED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: