1732103 - Webextension popups are difficult to debug, they close when I click anything in the Inspector in about:devtools-toolbox

Status: ASSIGNED

(DevTools :: about:debugging, enhancement, P2)

Description

[petr.laskevic]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:94.0) Gecko/20100101 Firefox/94.0

Steps to reproduce:

Load a temporary addon which has a toolbar button
Click Inspect to open about:devtools-toolbox
Switch to about:devtools-toolbox tab
Click the toolbar button so the popup opens
Click anywhere in the Inspector

Actual results:

The popup is closed, making inspecting it impossible. Same when I open two Firefox windows and in one of them select about:devtools-toolbox tab and in the other open the popup.

Expected results:

Firefox recognises about:devtools-toolbox is opened and does not close the popup. The addon is made quicker and easier.

Comment 1

[Release mgmt bot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'DevTools::Inspector' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Julian Descottes [:jdescottes]]

Can you try the "Disable popup autohide" option from the Toolbox menu?
This should allow you to debug your popup.

Comment 3

[Julian Descottes [:jdescottes]]

Attachment: image.png

Comment 4

[petr.laskevic]

(In reply to Julian Descottes [:jdescottes] from comment #2)

Can you try the "Disable popup autohide" option from the Toolbox menu?
This should allow you to debug your popup.

I will next time. How about making that option default though when debugging extensions (when the active tab is about devtools)? It makes no sense to have about devtools opened and opening the extension popup only for it to close when I click something in about devtools.

Comment 5

[Julian Descottes [:jdescottes]]

(In reply to petr.laskevic from comment #4)

I will next time. How about making that option default though when debugging extensions (when the active tab is about devtools)? It makes no sense to have about devtools opened and opening the extension popup only for it to close when I click something in about devtools.

I agree that's something which comes up regularly, and we should try to implement it.
Let's repurpose this bug as an enhancement.

Basically:

• disable popup autohide when an about:devtools-toolbox tab debugging a webextension is focused
• enable it again when the tab loses the focus or is closed

Some additional tasks that could be handled in follow up bugs:

• add some UI feedback/indicator to show that popup autohide is disabled (either a message or a toolbox icon?)
• make it so that popup autohide only swallows clicks in DevTools UI, so that you could still dismiss the dialogs. Right now if you disable popup autohide, it's quite difficult to dismiss a popup (it's the whole point, but still). Not sure if that is feasible

Comment 6

[Alexandre Poirot [:ochameau]]

It would have been nice if we could contribute to this platform code:
https://searchfox.org/mozilla-central/rev/00e504083572d47f2199168f0f79c75c0ddaefb3/layout/xul/nsXULPopupManager.cpp#282-296

bool nsXULPopupManager::Rollup(uint32_t aCount, bool aFlush,
                               const LayoutDeviceIntPoint* pos,
                               nsIContent** aLastRolledUp) {
  if (aLastRolledUp) {
    *aLastRolledUp = nullptr;
  }

  // We can disable the autohide behavior via a pref to ease debugging.
  if (StaticPrefs::ui_popup_disable_autohide()) {
    // Required on linux to allow events to work on other targets.
    if (mWidget) {
      mWidget->CaptureRollupEvents(nullptr, false);
    }
    return false;
  }

So that instead of blocking all rollups, we would only block rollups when they relate to a mouse click/wheel done from the DevTools document.
I tried to browse the various interfaces/data we have access to from this function (mostly nsIWidget and LayoutDeviceIntPoint),
but unfortunately, I've been unable to come up with anything which would help me know on which Document/Element/nsIFrame we clicked on.

I then briefly looked at the platform specific callsites:
https://searchfox.org/mozilla-central/rev/00e504083572d47f2199168f0f79c75c0ddaefb3/widget/gtk/nsWindow.cpp#7310-7365
https://searchfox.org/mozilla-central/rev/00e504083572d47f2199168f0f79c75c0ddaefb3/widget/cocoa/nsChildView.mm#2381-2455
But no luck either.

Emilio, would you have any suggestion ? I saw you "recently" helped around this disable_autohide logic in bug 1584218.
The idea is to avoid hidding popups when interacting with DevTools. Otherwise any other interaction would hide them.
I'd be up writing the patch if you have suggestion of where and how to identify the events relate to the DevTools document.

Comment 7

[Julian Descottes [:jdescottes]]

Note that we might still need the current behavior when debugging a popup from the Browser Toolbox, but for about:debugging scenarios this could be useful

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Alexandre Poirot [:ochameau] from comment #6)

So that instead of blocking all rollups, we would only block rollups when they relate to a mouse click/wheel done from the DevTools document.
I tried to browse the various interfaces/data we have access to from this function (mostly nsIWidget and LayoutDeviceIntPoint),
but unfortunately, I've been unable to come up with anything which would help me know on which Document/Element/nsIFrame we clicked on.

That seems reasonable, potentially, but it's a bit tricky, as you've noted... Actuayy, it might be easier to implement for stuff like the browser toolbox...

nsXULPopupManager::Rollup already gets the mouse position when available. The thing you don't have is the widget targeted by that, but the caller could provide that.

So you could have an extra nsIWidget* parameter from the caller and then change the disable_autohide check to do something like:

if (disable_autohide || ShouldAvoidRollupForDevTools(aTargetWidget, pos)) {
  // ... avoid rollup. Maybe only if the widgets rolled up are not descendants of aTargetWidget or something?
}

And ShouldAvoidRollupForDevTools would be something like:

static bool ShouldAvoidRollupForDevTools(nsIWidget* aWidget, LayoutDeviceIntPoint* aPos) {
  if (!aPos || !aWidget) {
    return false;
  }
  nsIWidgetListener* listener = aWidget->GetWidgetListener();
  if (!listener) {
    return false;
  }
  PresShell* ps = listener->GetPresShell();
  if (!ps) {
    return false;
  }
  nsIFrame* rootFrame = ps->GetRootFrame();
  if (!rootFrame) {
    return false;
  }
  // aPos seems screen-relative, at a glance.
  LayoutDeviceIntPoint widgetRelativePos = *aPos - aWidget->WidgetToScreenOffset();
  const nsPoint coords = nsLayoutUtils::GetEventCoordinatesRelativeTo(aWidget, widgetRelativePos, RelativeTo{rootFrame});
  // Here you could hit-test with `nsLayoutUtils::GetFramesForArea` or what not, make a decision based on what you hit.
}

I think such a thing could work, but it's quite the special-case. Does that help?

Comment 9

[Alexandre Poirot [:ochameau]]

Thanks a lot, this is super helpful! I'll try to shape something based on this.

Comment 10

[Alexandre Poirot [:ochameau]]

Here is a patch to demonstrate what we could do.

I'm using the document URI to identify the DevTools Toolbox, it is a bit gross, I'll try to find something more elegant.
I'd also need to enable this only when we have an active WebExtension toolbox.
May be a flag should be set on the toolbox document to only ignore events made on WebExtension toolbox, which would flag its document.

Otherwise all credits belongs to Emilio, thanks again for the help!

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1732103 - [devtools] Keep the extension popups opened when interacting with DevTools.

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1732103 - [devtools] Use mediator to find where we clicked.

For some reason, the window mediator is unable to iterate over all windows,
while GetMostRecentWindow works...

But all this starts to be quite complex, I'll investigate some other options.