1412364 - Incorrect panel alignmentPosition returned in PanelMultiView

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P4)

Description

[Sam Foster [:sfoster] (he/him)]

In the popupshowing handler in PanelMultiView, this._panel.alignmentPosition is always "before_end", even when the anchor is positioned such that the popup will actually be positioned above the anchor rather than below.

Comment 1

[Sam Foster [:sfoster] (he/him)]

Neil, any idea why alignmentPosition might always be returning "before_end" here: http://searchfox.org/mozilla-central/source/browser/components/customizableui/PanelMultiView.jsm#1011

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

before_end is still correct in that case. In this case it is "flipping" due to space constraints. You would have to check the "flip" attribute: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/panel.flip

Comment 3

[Neil Deakin]

When I open the panel the alignment is after_end. Is there some specific steps I am supposed to be looking to see this issue?

Comment 4

[Sam Foster [:sfoster] (he/him)]

Ah, misunderstanding/typo in comment #1 is not helping with the confusion here! It should read "after_end". 

Neil, yeah, move the browser window down to the bottom of the screen, so that e.g. the hamburger menu will open above the anchor rather than below it. In this case, I'm logging "after_end" even though as the popup opens above the anchor, I would expect to have gotten "before_end"? 

Here's the actual problem I'm looking to solve: That PanelMultiView code needs to set a maxHeight on the panel, and the value we calculate needs to depend on whether the popup will be shown above or below the anchor. We check _panel.alignmentPosition to know this.

Comment 5

[Sam Foster [:sfoster] (he/him)]

(In reply to Sam Foster [:sfoster] from comment #4)
> Neil, yeah, move the browser window down to the bottom of the screen, so
> that e.g. the hamburger menu will open above the anchor rather than below
> it. In this case, I'm logging "after_end" even though as the popup opens
> above the anchor, I would expect to have gotten "before_end"? 

Neil, I think I can answer some of this now. The code in question is in a handler for popupshowing and alignmentPosition hasn't been calculated yet. If I check it in a handler for popuppositioned, it gives me the expected value. Thanks for the calculated delay enabling me to figure this out for myself :)

Which means that the testBrowserActionMenuResizeBottomArrow in browser_ext_browserAction_popup_resize.js was giving us a false positive before. And to land the maxHeight changes, I'm not sure we can defer fixing this alignmentPosition issue. 

So I guess the next question is to find out how/if this worked in the old binding, before this PMV implementation. And what the consequences of moving some of this stuff into a popuppositioned handler would be. Mike, can I ask you to think about that so we can compare notes?

Comment 6

[Mike de Boer [:mikedeboer]]

(In reply to Sam Foster [:sfoster] from comment #5)
> Neil, I think I can answer some of this now. The code in question is in a
> handler for popupshowing and alignmentPosition hasn't been calculated yet.
> If I check it in a handler for popuppositioned, it gives me the expected
> value. Thanks for the calculated delay enabling me to figure this out for
> myself :)

O my, yeah, changing over to 'popuppositioned' seems more than fine to me!

> Which means that the testBrowserActionMenuResizeBottomArrow in
> browser_ext_browserAction_popup_resize.js was giving us a false positive
> before. And to land the maxHeight changes, I'm not sure we can defer fixing
> this alignmentPosition issue. 

I'm already a bit sad that this isn't making 57, so let's get it landed & shipped as soon as we possibly can.

> So I guess the next question is to find out how/if this worked in the old
> binding, before this PMV implementation. And what the consequences of moving
> some of this stuff into a popuppositioned handler would be. Mike, can I ask
> you to think about that so we can compare notes?

Not sure, but I have the feeling that we're getting many more eyeballs on the panels and their various scenarios of interaction lately, from UX, QA and engineering. Also, the Photon design focuses a lot more on their usability.
As an aside, that's also why I don't 'feel' it when people say 'just backout the new panel design and animation, it was working before!'.

Anyway, yes, the current panel animation that shows _two_ panels at the same time whilst animating is pushing the limits of the design and uncovers a couple of flaws that we were able to iron over using a simpler design. ('Simpler' solely from an engineering perspective, not from a UX perspective.)

Comment 7

[Sam Foster [:sfoster] (he/him)]

Attachment: popup-opened-below.png

Screenshot of how this bug manifests itself. Instead of opening the popup above the anchor, it opens below, where there the height is constrained and you have to scroll through the items.

Comment 8

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1412364 - set panel height in popuppositioned, after a layout flush

* Defer the maxHeight calculation until popuppositioned, waiting for the next layout flush
* Defer setting panelopen attribute to start the height transition until after height calculations
* Adjust (down one) the expected uninterruptble reflows in the appmenu reflow test

Review commit: https://reviewboard.mozilla.org/r/196040/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/196040/

Comment 9

[Sam Foster [:sfoster] (he/him)]

Manual testing of attachment 8924798 [details] is promising. I don't see any flicker as a result of both moving the sizing into popuppositioned and waiting for a layout flush. Try results also tentatively promising: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a1e33f621c74cf6c11d6ab7e409835744ca37d8e&selectedJob=141857370 - have to handle the rejection case and check that's not masking more serious errors.

Comment 10

[Sam Foster [:sfoster] (he/him)]

This looks like the minimum changes necessary to get the expected behavior. See Comment #4 for STR. The main concern is that these changes will introduce some flicker as you open e.g. the appmenu or other PMV instance. But I've not seen any problem so far - bears careful checking though.  

I've left the blockinboxworkaround workaround as-is in the spirit of keeping the scope of this patch down. But it might be worth checking that is still necessary and still the right solution with these changes.

Comment 11

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8924798 [details]
Bug 1412364 - set panel height in popuppositioned, after a layout flush

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196040/diff/1-2/

Comment 12

[Mike de Boer [:mikedeboer]]

Comment on attachment 8924798 [details]
Bug 1412364 - set panel height in popuppositioned, after a layout flush

https://reviewboard.mozilla.org/r/196040/#review202028

Thanks for working on this, Sam! This looks good to me, but I'd like to take another look at the refactored version.

::: browser/base/content/test/performance/browser_appmenu_reflows.js:46
(Diff revision 2)
> -      "handleEvent@resource:///modules/PanelMultiView.jsm",
> +      "handleEvent/</<@resource:///modules/PanelMultiView.jsm",
> +      "promise callback*handleEvent/<@resource:///modules/PanelMultiView.jsm",
> +      "EventListener.handleEvent*handleEvent@resource:///modules/PanelMultiView.jsm",
>        "openPopup@chrome://global/content/bindings/popup.xml",
>      ],
> -
> +    times: 6, // This number should only ever go down - never up.

Woah, this is an unexpected perk!

::: browser/components/customizableui/PanelMultiView.jsm:992
(Diff revision 2)
>          // every time the popup closes, which is why we have to set it each time.
>          this._panel.autoPosition = false;
> -        if (this.panelViews && !this.node.hasAttribute("disablekeynav")) {
> -          this.window.addEventListener("keydown", this);
> -          this._panel.addEventListener("mousemove", this);
> +        // the rest of the sizing needs to wait for popuppositioned.
> +        // but we only need to do this once (popuppositioned can fire repeatedly
> +        // while the popup is shown)
> +        this._panel.addEventListener("popuppositioned", () => {

Sorry to push this onto you, but can you please refactor this a wee bit into an `async _onPopupShowing()`?

Comment 13

[Sam Foster [:sfoster] (he/him)]

Update on this: I'm still trying to get ahead of the browser_appmenu_reflows test failures. After updating the stack signatures, I'm still seeing new reflows. One thing I noticed is that setting this._panel.autoPosition = false in the popupshowing handler seems to be having no effect - by the time popuppositioned is fired, it appears to have been reset to true. So I get a bunch of further popuppositioned events as the subviews open and the mainview changes size, and each hits adjustArrowPosition which causes flush/reflow. Looking at nsMenuPopupFrame.cpp, this does indeed get reset when the popup is shown, but not elsewhere so while this observation explains the test results I'm seeing, it isn't making much sense yet...

Comment 14

[:Paolo Amadini]

I just stumbled upon this bug. Waiting for a layout flush does seem fragile to me, unless for some reason promiseLayoutFlushed guarantees the callback happens before we try to paint the panel without the maximum height.

In particular, I'd expect a flicker for cases where a scrollbar would normally appear in the main view. You can test this by placing the main menu anchor in the middle of the screen with a lower screen resolution, or maybe also by placing the non-default History button into the navigation toolbar. Also note that the panel timing is quite different between platforms, so this should be tested on Windows, Mac, and Linux separately.

Since the current logic is shared between Photon and non-Photon panels, and it worked fine when I implemented it in bug 1367776, can we identify the regressing bug? It may be a change that is easier to revert in the panel code, rather than working around it in the front-end.

Comment 15

[:Paolo Amadini]

Also, if I remember correctly, when working on bug 1367776 I tried to place the code in the "popuppositioned" handler, but either it was too late or it was equivalent to "popupshowing".

Comment 16

[Sam Foster [:sfoster] (he/him)]

(In reply to :Paolo Amadini from comment #15)
> Since the current logic is shared between Photon and non-Photon panels, and
> it worked fine when I implemented it in bug 1367776, can we identify the
> regressing bug? It may be a change that is easier to revert in the panel
> code, rather than working around it in the front-end.

Yeah, finding the regression is probably a better first step here. I'll track it down. 

> Also, if I remember correctly, when working on bug 1367776 I tried to place
> the code in the "popuppositioned" handler, but either it was too late or it
> was equivalent to "popupshowing".

That is curious because, as I understand it, alignmentPosition isn't guaranteed to return the correct value until popuppositioned is dispatched. It is exactly the work of figuring out where the popup will best fit (above/below anchor) before painting it which makes this event necessary.

Comment 17

[Sam Foster [:sfoster] (he/him)]

I tracked down 2 stages to this regression. 

STR: 
* open new window, 
* move window to lower part of screen, 
* open app (hamburger) menu.

Expected: app menu opens above toolbar, showing all/most of the items without scrolling.
Actual: app menu opens below the toolbar, in a short/scrolling menu

This originally regressed in Bug 1401991. 

The regression there is fairly subtle, because after the first time the app menu is opened, the positioning gets corrected. So if you open the app menu first, and then move the window down you'll get the expected behavior. 

The current behavior in nightly is worse however: 

STR: 
* use browser, including open app (hamburger) menu.
* move window to lower part of screen, 
* open app (hamburger) menu.
(repeat as necessary)

Expected: when window is positioned near the bottom of the screen, the app menu opens above the toolbar, showing all/most of the items without scrolling.
Actual: app menu opens below the toolbar, in a short/scrolling menu

This was regressed by bug 1356674

I'm not sure what the path forward is here. AFAICT, my diagnosis of using alignmentPosition in popupshowing remains valid, but I would need to better understand how this worked before the pretty major changes in Bug 1401991. (Simply backing that out is not an option)

Comment 18

[Mike de Boer [:mikedeboer]]

(In reply to :Paolo Amadini from comment #15)
> Also, if I remember correctly, when working on bug 1367776 I tried to place
> the code in the "popuppositioned" handler, but either it was too late or it
> was equivalent to "popupshowing".

I'd like to get a bit of proof for this (the 'too late' part), before we completely disregard this approach, because it's quite sensible to me.

Comment 19

[Neil Deakin]

The alignmentPosition should have the right value during and after the popupshowing event as long as you don't change the panel in some manner afterwards. Changing something in or of the panel (such as by setting the maximum height) can cause to panel to get repositioned such that the alignmentPosition gives a different value.

One caveat is that the operating system might move the popup on its own when the popup is finally shown, but I don't think that is an issue here.

Comment 20

[:Paolo Amadini]

I don't know about the impact of bug 1401991, but we might be leaving some CSS property behind that simply causes the panel to have something like a zero or small height at first, and this might influence the initial positioning. Neil, does that sound plausible? Also, any idea of how removing the code that positions the arrow in "popupshowing" in bug 1356674 could have regressed the behavior?

I also took a look at the popup opening code, and using the "popuppositioned" event also looks good to me, if that happens to be related and would solve the issue.

The code in the event handler must still be executed synchronously though, without waiting for a layout flush. This is because after the event has been fired the popup layout code immediately checks if the popup should be re-positioned before the fade-in animation starts or the popup is shown. If this is done later, a scheduled paint will occur in-between, and this would very likely cause a flicker, which would be visible in particular if the panel opens without animating.

Comment 21

[:Paolo Amadini]

(In reply to :Paolo Amadini from comment #20)
> Also, any idea of how removing
> the code that positions the arrow in "popupshowing" in bug 1356674 could
> have regressed the behavior?

Note that I narrowed this down to just reading the alignmentPosition property.

Comment 22

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

* WIP, straw man for a potential approach
* All-sync event handlers - no waiting for layout flushes etc.
* Let popup code meaure and place the panel without maxHeight, this ensures alignmentPosition is a reasonable value
* Assign maxHeight from a popuppositioned handler
* Refactor to move the maxHeight calculation into a method on PMV
* Reflow tests are expected to break, I'm just exploring possibilities at this point

Review commit: https://reviewboard.mozilla.org/r/198304/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/198304/

Comment 23

[Sam Foster [:sfoster] (he/him)]

Mike, do you know where the attribute blockinboxworkaround gets set? I don't see it in dxr/searchfox. and blame isn't helping - is it still applicable?

Comment 24

[Sam Foster [:sfoster] (he/him)]

Something like this is what I'm taking away from the discussion so far: 

> Created attachment 8927097 [details]
> Bug 1412364 - defer applying maxHeight until popup is positioned
> 
> * WIP, straw man for a potential approach
> * All-sync event handlers - no waiting for layout flushes etc.
> * Let popup code meaure and place the panel without maxHeight, this ensures
> alignmentPosition is a reasonable value
> * Assign maxHeight from a popuppositioned handler
> * Refactor to move the maxHeight calculation into a method on PMV
> * Reflow tests are expected to break, I'm just exploring possibilities at
> this point
> 
> Review commit: https://reviewboard.mozilla.org/r/198304/diff/#index_header

I'm not sure what to make of this: 

> The alignmentPosition should have the right value during and after the
> popupshowing event as long as you don't change the panel in some manner
> afterwards. Changing something in or of the panel (such as by setting the
> maximum height) can cause to panel to get repositioned such that the
> alignmentPosition gives a different value.

... as that is exactly what we need to do. However, the maxHeight is there to cap the height of subviews, and AIUI shouldn't actually change the height of the panel?

Comment 25

[:Paolo Amadini]

(In reply to Sam Foster [:sfoster] from comment #23)
> Mike, do you know where the attribute blockinboxworkaround gets set? I don't
> see it in dxr/searchfox. and blame isn't helping - is it still applicable?

I can answer this - I had to implement this workaround because of the moveable buttons in the non-Photon application menu, but now that the old code has been removed the workaround can be removed as well.

Comment 26

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198304/diff/1-2/

Comment 27

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198304/diff/2-3/

Comment 28

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198304/diff/3-4/

Comment 29

[Sam Foster [:sfoster] (he/him)]

(In reply to :Paolo Amadini from comment #25)
> I can answer this - I had to implement this workaround because of the
> moveable buttons in the non-Photon application menu, but now that the old
> code has been removed the workaround can be removed as well.

This got tidied up in bug Bug 1388029

Neal: I have a new patch attached that seems to both make sense (to me at least) and is passing tests locally. It uses all-sync event handlers, moves the maxHeight calculation into the popuppositioned handler (only when state=="showing"). By waiting until popuppositioned, the alignmentPosition gives the correct value and maxHeight is calculated correctly. I don't think we need further info at this point.

Comment 30

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198304/diff/4-5/

Comment 31

[:Paolo Amadini]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

https://reviewboard.mozilla.org/r/198304/#review206408

Looks good to me, thanks! I haven't tested it, but I assume you did on all platforms and there are no glitches.

::: browser/components/customizableui/PanelMultiView.jsm:1036
(Diff revision 5)
> -        // To go from the maximum height of the panel to the maximum height of
> -        // the view stack, we need to subtract the height of the arrow and the
> -        // height of the opposite margin, but we cannot get their actual values
> -        // because the panel is not visible yet. However, we know that this is
> -        // currently 11px on Mac, 13px on Windows, and 13px on Linux. We also
> -        // want an extra margin, both for visual reasons and to prevent glitches
> -        // due to small rounding errors. So, we just use a value that makes
> +        // Bug 941196 - The panel can get taller when opening a subview. Disabling
> +        // autoPositioning means that the panel won't jump around if an opened
> +        // subview causes the panel to exceed the dimensions of the screen in the
> +        // direction that the panel originally opened in. This property resets
> +        // after each popuppositioned event, and every time the popup closes,
> +        // which is why we have to set it each time.
> +        this._panel.autoPosition = false;

I'm wondering if this is still required, as we set the maxHeight before the panel opens. If so, the reason stated in the comment should be updated.

Comment 32

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

https://reviewboard.mozilla.org/r/198304/#review206420

::: browser/components/customizableui/PanelMultiView.jsm:1036
(Diff revision 5)
> -        // To go from the maximum height of the panel to the maximum height of
> -        // the view stack, we need to subtract the height of the arrow and the
> -        // height of the opposite margin, but we cannot get their actual values
> -        // because the panel is not visible yet. However, we know that this is
> -        // currently 11px on Mac, 13px on Windows, and 13px on Linux. We also
> -        // want an extra margin, both for visual reasons and to prevent glitches
> -        // due to small rounding errors. So, we just use a value that makes
> +        // Bug 941196 - The panel can get taller when opening a subview. Disabling
> +        // autoPositioning means that the panel won't jump around if an opened
> +        // subview causes the panel to exceed the dimensions of the screen in the
> +        // direction that the panel originally opened in. This property resets
> +        // after each popuppositioned event, and every time the popup closes,
> +        // which is why we have to set it each time.
> +        this._panel.autoPosition = false;

I'm not sure I 100% understand your concern here, but here is what I know: Setting autoPosition=false is still very necessary. Without it we get extra, spurious events which needlessly run the arrow positioning function and its reflows. We (frontend) know the panel will always fit because we've calculated and set maxHeight, but the popup manager does not and still attempts to re-position it unless we set autoPosition=false here. 

I made a minor update to the comment (in the diff you reviewed) to be clearer. What else do you think needs spelling out here?

Comment 33

[Sam Foster [:sfoster] (he/him)]

Flagging ni? for comment #32. 

I have tested across mac/windows/linux. I've seen a glitch (slight stutter in the opening) with this patch maybe 1/20 times, and only on the very first time the app menu is used in a session. I'm guessing at the frequency - I saw it earlier on windows, but I've been unable to reproduce it again this morning. I'm not able to say if this is a regression because of the infrequency abd I know I saw similar behavior when I was working on the panel animations. So I'm inclined to land this and keep an eye on it. Maybe get some more eyes on it before requesting uplift for 58.

Comment 34

[:Paolo Amadini]

(In reply to Sam Foster [:sfoster] from comment #32)
> Setting autoPosition=false is still very necessary. Without it we get extra,
> spurious events which needlessly run the arrow positioning function and its
> reflows. We (frontend) know the panel will always fit because we've
> calculated and set maxHeight, but the popup manager does not and still
> attempts to re-position it unless we set autoPosition=false here. 
> 
> I made a minor update to the comment (in the diff you reviewed) to be
> clearer. What else do you think needs spelling out here?

Well, the description you made here is much clearer, so I'd just replace most of the existing comment with it :-)

In fact, the part of the existing comment about subviews causing the panel to exceed the dimensions of the screen is not true anymore, because their height is now limited before they are opened.

(In reply to Sam Foster [:sfoster] from comment #33)
> So I'm inclined to land this and keep an eye on it.

Thanks for describing the testing in detail! Given the results, landing sounds good to me.

Comment 35

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/198304/diff/5-6/

Comment 36

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8927097 [details]
Bug 1412364 - defer applying maxHeight in PanelMultiView until popup is positioned.

https://reviewboard.mozilla.org/r/198304/#review206972

::: browser/components/customizableui/PanelMultiView.jsm:1036
(Diff revision 5)
> -        // To go from the maximum height of the panel to the maximum height of
> -        // the view stack, we need to subtract the height of the arrow and the
> -        // height of the opposite margin, but we cannot get their actual values
> -        // because the panel is not visible yet. However, we know that this is
> -        // currently 11px on Mac, 13px on Windows, and 13px on Linux. We also
> -        // want an extra margin, both for visual reasons and to prevent glitches
> -        // due to small rounding errors. So, we just use a value that makes
> +        // Bug 941196 - The panel can get taller when opening a subview. Disabling
> +        // autoPositioning means that the panel won't jump around if an opened
> +        // subview causes the panel to exceed the dimensions of the screen in the
> +        // direction that the panel originally opened in. This property resets
> +        // after each popuppositioned event, and every time the popup closes,
> +        // which is why we have to set it each time.
> +        this._panel.autoPosition = false;

I updated the comment to explain what's going on with the autoPosition property. I removed the old reference to bug 941196 as I don't think that adds much value at this point.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d8321357c4a7
defer applying maxHeight in PanelMultiView until popup is positioned. r=Paolo

Comment 38

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/d8321357c4a7

Comment 39

[Sam Foster [:sfoster] (he/him)]

It would be great to uplift to 58, but as the fix addresses edge-case behavior I'm not confident just sitting on nightly will smoke out any bugs in the short team.

I'd like to get this put through its paces - testing menu/panel behavior when the browser window is moved to top and the bottom of the screen; with a single display and a dual display; and with the browser in the primary and secondary display; and when the browser window spans the 2 displays. 

As I looked for the cause of this issue with mozregression, I also noticed a difference in behavior between the very first time the main "hamburger" panel was opened vs. subsequent use, in the initial window and when opening from new browser windows. 

In each of these cases, I'd like to confirm that the panel opens in the expected position above or below the toolbar, and without introducing new flickering or other jank.

Comment 40

[Sam Foster [:sfoster] (he/him)]

Flagging for qe-verify, see comment #39 for details.

Comment 41

[Ryan VanderMeulen [:RyanVM]]

Given where we are in the cycle, I think this is wontfix for 58 at this point. Feel free to set the status to affected and nominate it for approval if you feel strongly otherwise, though.

Comment 42

[Oana Botisan, Desktop QA]

I managed to reproduce the bug using an older version of Nightly (2017-10-27) on Windows 10 x64. I took in consideration the details from comment 39.
I retested everything on latest Nightly 60.0a1 and beta 59.0b5 using Windows 10 x64, macOS 10.13 and Ubuntu 16.04 x64, but the bug is not reproducing anymore. The panel now opens underneath the toolbar if it's positioned on the upper half of the screen and it opens above the toolbar if it's positioned on the lower half of the screen. I think this bug is fixed.