1842366 - Support rendering chrome feature callouts in <panel> elements

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P1)

Description

[Shane Hughes [:aminomancer]]

This is necessary to get the callout to reposition on any kind of reflow. We currently only care about resize and toggle events, which works okay in content documents not subject to much change, but in the firefox UI, the anchor element could move for many other reasons, like a sibling appearing or disappearing and affecting the layout. This will be especially critical for anchoring callouts to the shopping icon in the address bar.

I have a rough draft for preview purposes

Comment 1

[Shane Hughes [:aminomancer]]

Attachment: Bug 1842366 - Part 2: Allow rendering Feature Callout in panels. r?jprickett,emilio

Comment 2

[Shane Hughes [:aminomancer]]

There's one more critical question for this feature. Instead of relying on our own system for positioning callouts, this new patch takes advantage of xul popup alignment, by rendering the callout in a <panel> element, and passing it a standard position/alignment string when showing it. But I'm not sure if that alignment system includes any way to align popup center to anchor center. It doesn't seem to.

Emilio, I thought you might be able to point us in the right direction. We need to basically recreate some of the logic from the old arrowpanels for these messaging callouts, but with the option to have the arrow centered, as it is in the screenshots. That is, the arrow's center is aligned to the callout's center, and the callout's center is aligned to the anchor's center.

I think arrowpanels only supported e.g. aligning popup left to anchor center. And unless I'm misunderstanding something, it looks like that's how the code works too. So I'm guessing this will require some additional C++ work to support these positions. It may not be urgent, since I think our planned uses so far will all involve standard popup positions.

My draft includes a test message in FeatureCalloutMessages.sys.mjs that specifies bottomcenter topright. The outcome can be seen by just opening a tab and then closing it. Basically what we'd want is the ability to specify bottomcenter topcenter (as an example) and have it align center-to-center.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Shane Hughes [:aminomancer] from comment #2)

My draft includes a test message in FeatureCalloutMessages.sys.mjs that specifies bottomcenter topright. The outcome can be seen by just opening a tab and then closing it. Basically what we'd want is the ability to specify bottomcenter topcenter (as an example) and have it align center-to-center.

I cannot apply this cleanly, any chance you can provide an updated patch?

Emilio, I thought you might be able to point us in the right direction. We need to basically recreate some of the logic from the old arrowpanels for these messaging callouts, but with the option to have the arrow centered, as it is in the screenshots. That is, the arrow's center is aligned to the callout's center, and the callout's center is aligned to the anchor's center.

I think arrowpanels only supported e.g. aligning popup left to anchor center. And unless I'm misunderstanding something, it looks like that's how the code works too. So I'm guessing this will require some additional C++ work to support these positions. It may not be urgent, since I think our planned uses so far will all involve standard popup positions.

That's right. If the panel width is fixed you can get around it by using negative margins I guess (so using bottomcenter topleft, then using something like margin-left: calc(var(--panel-width) / 2) or so.

But yeah if you want center-to-center alignment you need to add code to nsMenuPopupFrame to deal with it and also decide what's the right thing to do when sliding or shrinking the popup so it fits on screen.

Comment 4

[Shane Hughes [:aminomancer]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

(In reply to Shane Hughes [:aminomancer] from comment #2)

My draft includes a test message in FeatureCalloutMessages.sys.mjs that specifies bottomcenter topright. The outcome can be seen by just opening a tab and then closing it. Basically what we'd want is the ability to specify bottomcenter topcenter (as an example) and have it align center-to-center.

I cannot apply this cleanly, any chance you can provide an updated patch?

Yeah, sorry about that 🤦‍♀️ I accidentally set the wrong parent revision. It should be working now.

Comment 5

[Shane Hughes [:aminomancer]]

Attachment: Bug 1842366 - Part 1: Support center alignment for XUL popups. r=emilio

Comment 6

[Shane Hughes [:aminomancer]]

I think we also need to do something to close the callout if another panel opens in the same window. Otherwise you could have a panel stacking over another panel.

Comment 7

[Shane Hughes [:aminomancer]]

I disabled a test for win11 here that was keeping bug 1804349 open. I had to do it in this bug because my patch changes the failure message, so taskcluster thinks it's a new failure which I guess could cause a backout. But ideally we should figure out the cause of the win11 issue so we can re-enable it on win11.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c9e538ad8526
Part 1: Support center alignment for XUL popups. r=emilio
https://hg.mozilla.org/integration/autoland/rev/bcc147043981
Part 2: Allow rendering Feature Callout in panels. r=jprickett,desktop-theme-reviewers,dao

Comment 9

[Atila Butkovits]

Backed out for causing Xpcshell failures at test_PanelTestProvider.js.

Backout link: https://hg.mozilla.org/integration/autoland/rev/2d0c68d7097bd36dc5ec88db7a8c450ce6a5ccb1

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=bcc1470439818e29a38c29432af9fd130b8ec931&selectedTaskRun=fdwUbNDARQSP0WfRPv3kJQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=427543413&repo=autoland&lineNumber=5610

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/afbedb0bcdca
Part 1: Support center alignment for XUL popups. r=emilio
https://hg.mozilla.org/integration/autoland/rev/6dd1e008f3df
Part 2: Allow rendering Feature Callout in panels. r=jprickett,desktop-theme-reviewers,dao

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/afbedb0bcdca
https://hg.mozilla.org/mozilla-central/rev/6dd1e008f3df