Open Bug 1198432 Opened 9 years ago Updated 1 year ago

marionette.screenshot can't get hold of popups

Categories

(Remote Protocol :: Marionette, defect, P3)

Product:
Remote Protocol
Component:
Marionette
Type:
defect

Tracking

(Not tracked)

People

(Reporter: Pike, Unassigned)

References

(Depends on 1 open bug,
URL
)

Details

Getting a bug on file.

I'm working on a variant of the firefox-ui-tests, trying to get a screenshot of the security popup. https://github.com/mozilla/firefox-ui-tests/blob/master/firefox_ui_tests/functional/security/test_dv_certificate.py#L43 is the line after which I'd like to go in and take a screenshot.

In various ways, the popup evades the screenshot.

Edwin mentioned that he saw a dev with a build that can do that. Henrik knows that this is a problem.

Given the importance of that pop-up in the 42 campaign, we have an opportunity to scale visual QA on the localizations of this UI if we could automate screenshots of it.

Filing, though filing late. Didn't realize the potential until now, sorry.
Edwin, can you remember who was that dev with the popup screenshot functionality? I would kinda love to get this bug fixed!

Given that this might be a Marionette issue, lets move it over to its component for now.
Component: XUL Widgets → Marionette
Product: Toolkit → Testing
Flags: needinfo?(edwong)
It would be good if someone could attach a screenshot and a minified, reproducible test case to this bug.
this is what Jared H. said: 
basically, you'd need to listen for whatever the "about to close" event is for whatever the panel subclass is that implements the doorhanger, then cancel the event (if it's cancelable) at least, that's how I implemented the pref for universal-search

maybe this is a feature request for a pref to do this without an add-on.
Flags: needinfo?(edwong)
Hm, Edwin this answer doesn't feel connected to the question in that bug. This is about a screenshot of popup elements and not something about canceling a close request for a popup. Maybe it was a different topic?
Actually, I think Edwin got the right pointer. My suspicion is that taking a screenshot messes with the focus, which it probably shouldn't. So that's one part of the bug, but I had no idea how to actually fix that.

But his pointer made me hunt, and I found https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#7196.

I'll try to mess with the noautohide attribute today :-)
Oh! You are right, then! It looks indeed promising. Lets hope that this idea will help. Thank you Axel for diving into that problem and Edwin for the pro tip.
Depends on: 1199183
To add why I filed bug 1199183:

I've added the noautohide to puppeteer, and then dug in, and screenshots still didn't work. But having the popup stay independent of focus allowed me to hook up a browser toolbox to the tested instance (how meta), and play around a bit. And changing the display: away from -moz-popup made the content show in the screenshot. Thus I filed a bug on layout.

Note, I also tried to use drawWidgetAsOnScreen, even that doesn't work, on my windows 7 VM. I.e. there it shows something, but not the popup. (on mac, it's just white, also, OK per comment in the IDL.)
I set the ui.popup.disable_autohide preference to true when writing tests for the door hanger notifications. It allows me to inspect the popups in browser toolbox without them disappearing. I found out about this feature here: https://developer.mozilla.org/en-US/docs/Tools/Browser_Toolbox#Debugging_popups
Yeah, that's half the answer I used in the end:

https://github.com/mozilla/firefox-ui-tests/compare/mozilla-central...Pike:l10n-tests?expand=1#diff-5a5eb75c022ec5349e22d60f86f64d2aR425 has that.

The other half is to use the popup as background for an in-browser element, which I did at https://github.com/mozilla/firefox-ui-tests/compare/mozilla-central...Pike:l10n-tests?expand=1#diff-7f5b4a1fb4d95301c4852903576731fcR73

Also in bug 1199183 comment 6.

Haven't actively looked into this in a while myself.
Is the right automation solution here to set ui.popup.disable_autohide as a recommended preference in https://searchfox.org/mozilla-central/rev/84cea84b12145d752e50ddca6be5462c38510e35/testing/marionette/server.js#60?
Flags: needinfo?(l10n)
Priority: -- → P5
I wouldn't know the pros and cons of that, sorry. Also, I haven't touched anything marionette in a long time.
Flags: needinfo?(l10n)

(In reply to Andreas Tolfsen ⦗:ato⦘ from comment #10)

Is the right automation solution here to set ui.popup.disable_autohide as a
recommended preference in
https://searchfox.org/mozilla-central/rev/
84cea84b12145d752e50ddca6be5462c38510e35/testing/marionette/server.js#60?

Olli, maybe you can help here or at least know someone who could give a qualified answer? Thanks.

Flags: needinfo?(bugs)

If the question is about ui.popup.disable_autohide, given it is false by default, wouldn't it be good to be able to run tests with ui.popup.disable_autohide false, no?

But which API is used for capturing screenshots? And at which point is screenshot taken?

Flags: needinfo?(bugs)

(In reply to Olli Pettay [:smaug] from comment #13)

If the question is about ui.popup.disable_autohide, given it is false by default, wouldn't it be good to be able to run tests with ui.popup.disable_autohide false, no?

We could leave it disabled but only flip the pref when taking a screenshot, or maybe opting-in for disabling auto-hide.

But which API is used for capturing screenshots? And at which point is screenshot taken?

It's using drawWindow() from CONTEXT_2D:
https://searchfox.org/mozilla-central/rev/cc280c4be94ff8cf64a27cc9b3d6831ffa49fa45/testing/marionette/capture.js#154

The screenshot is taken when an assertion fails or an exception is raised. So at this point nothing has been cleaned-up yet by Marionette.

Flags: needinfo?(bugs)

ah, screenshot is taken at that point. But why has the popup been closed?
Forcing popup to stay open longer changes behavior, and wouldn't test what we ship.

Flags: needinfo?(bugs)

Marionette doesn't close the popup, it's just not part of the screenshot. And that's the problem we are trying to figure out here.

Digging in the back of my brain:

The popup is torn off of it's parent window, so when the screenshot is taken, the underlying code thinks it's not part of the stuff it should cover.

I hacked around that by creating a div of the same size, and use the popup element as background, IIRC, with -moz-element().

yes, I don't think the drawWindow can really draw the popup, since the main page and popup live in separate OS level widgets and what not.
PresShell does have RenderNode, but looks like that is used only by drag-and-drop, and not exposed to JS.
-moz-element hack doesn't sound too horrible to me :)

I just want to give an update, and maybe get some more ideas in how to get this done. As it looks like this is not an actual issue for Marionette but all screenshot features in Firefox are affected.

Here some steps to reproduce:

  1. Open about:config and set ui.popup.disable_autohide to true (to keep the popup open for the click in step 5)
  2. Add the Take Screenshot button to the toolbar (you cannot use the context menu entry)
  3. Load data:text/html;charset=utf-8,%3Cselect%3E%3Coption%3EOption%201%3C/option%3E%3C/select%3E
  4. Click the select element
  5. Click the Take Screenshot toolbar button and select Save visible.

The screenshot preview after step 5 does not show the popup with Option 1. Other browsers like Chrome handle that scenario at least when testing through WebDriver (chromedriver).

Matt, is that something we can get fixed in general for screenshots (drawWindow0? I'm fairly sure that there are use cases when users want to have these popups included in the screenshot. If not would the -moz-element hack (whatever that was) be a solution that each of the take screenshot features would have to implement? Maybe it's time for a shared component... Thanks.

Flags: needinfo?(matt.woodrow)
Priority: P5 → P3
Summary: marionette.screenshot can't get hold of xul popup → marionette.screenshot can't get hold of popups

I'm not sure what the -moz-element hack is either, as far as I can tell it only draws the select element itself, not popup dropdown.

As Olli said, popups are separate OS windows, so there's no real way for drawWindow of the normal window to include content from the popup (which could be outside the bounds of the browser window).

I guess it could be possible to have a way to do a drawWindow of the popup, and then Marionette could figure out how to place that image relative to the main screenshot.

Flags: needinfo?(matt.woodrow)

(In reply to Matt Woodrow (:mattwoodrow) from comment #20)

I guess it could be possible to have a way to do a drawWindow of the popup, and then Marionette could figure out how to place that image relative to the main screenshot.

But that sounds more like a general screenshot related feature, which other implementations in the browser (--screenshot, via location bar / context menu, or devtools) could also benefit from. So my feeling is that this should be at least some shared code. What do you think?

Also is there a way to find out which popups a site has currently open? How could these be iterated?

Flags: needinfo?(matt.woodrow)

Interesting is that since Firefox 89 we are also not able to capture any content modal dialog anymore due to the redesign.

Select popups and content modal dialogs are different and would need different approaches and thus probably different bugs; it's probably easier to fix the content modal dialogs.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #23)

Interesting is that since Firefox 89 we are also not able to capture any content modal dialog anymore due to the redesign.

Did you actually check pre-89 behaviour? I don't really understand why the redesign would matter here. You must have already had a workaround for the content prompts because both 88 and 89 put the content modal dialogs in the parent process, so drawWindow on the child will not capture them either way.

Flags: needinfo?(hskupin)

You were correct. I've reopened the other bug, and we will continue with the discussion there.

Flags: needinfo?(hskupin)

For select popups, it looks like you can query the 'SelectParent' actor, and use that to query if there's currently a popup open, and hopefully its position relative to the outer window.

Unfortunately, we don't have a way to request a screenshot of these, since both of our screenshot entry points (WindowGlobalParent.drawSnapshot, CanvasRenderingContex2D.drawWindow) always capture a screenshot from the root element of a document, and we need to draw starting from the menupopup's widget/view.

Timothy, do you think it'd make sense to add an alternate version of drawSnapshot/drawWindow that took a XULPopupElement parameter, and let us call into PaintFrame using the popup view's frame?

Flags: needinfo?(matt.woodrow) → needinfo?(tnikkel)

Talked to Timothy yesterday and he agreed with comment 26. As such I filed bug 1743969 to get the missing platform feature implemented.

Flags: needinfo?(tnikkel)
Severity: normal → S3
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.