1782524 - Reposition Feature Callout if it overlaps parent element by more than 15%

Status: VERIFIED FIXED

(Firefox :: Messaging System, enhancement, P1)

Description

[Meg Viar [:mviar]]

In bug 1779423, the Feature Callout screens are configured to point to a parent element from the top, bottom, left, or right. The Feature Callout's basic positioning ensures it doesn't go offscreen. If the configured arrow position puts the callout in a position where it would go offscreen, it moves over to overlap the parent element. If the overlap is more than 15%, then the callout should move to another position. For example, if it was configured to point from the bottom but there's not enough room on top of the parent element for the callout, it should be positioned to point from the bottom. If there's not room there, then it should point from the left or right.

Part of this bug includes reexaming the default "margin" value of 15px used in FeatureCallout.mjs. Per figma the max overlap should be 15% of the parent. After chatting with design, we'll try just using a fixed 15% if that looks reasonable.

Comment 1

[Meg Viar [:mviar]]

Attachment: Bug 1782524 - Reposition Feature Callout if it overlaps parent element by more than 15%

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cbf8bf85c77
Reposition Feature Callout if it overlaps parent element by more than 15% r=Mardak

Comment 3

[Cosmin Sabou [:CosminS]]

Backed out for bc failures on browser_notification_dot.js.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&fromchange=5cbf8bf85c772ecacd933a4add72ec1beec482c4&searchStr=bc&tochange=0d1977cbd4ef689e23b6a23f126fe56b232c3328&selectedTaskRun=dhtOiD_jRjGE_oj-udBnzg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=387898344&repo=autoland
https://treeherder.mozilla.org/logviewer?job_id=387898675&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/0d1977cbd4ef689e23b6a23f126fe56b232c3328

Comment 4

[Meg Viar [:mviar]]

@Cosmin, thanks for flagging this and sorry it wasn't caught on try. There was an issue with not resetting the window size after changing it for a test. I've addressed it in the patch and will attempt to reland.

Try push with update: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cbc04ec707ab3157455af1817f9f77c3633dab2

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d022fd2c075
Reposition Feature Callout if it overlaps parent element by more than 15% r=Mardak

Comment 6

[Cosmin Sabou [:CosminS]]

(In reply to Meg Viar from comment #4)

@Cosmin, thanks for flagging this and sorry it wasn't caught on try. There was an issue with not resetting the window size after changing it for a test. I've addressed it in the patch and will attempt to reland.

Try push with update: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cbc04ec707ab3157455af1817f9f77c3633dab2

Unfortunately browser_feature_callout and browser_notification_dot did not run on this try push and we have to back it out again. browser_notification_dot is passing after the latest land but browser_feature_callout.js is permafailing with

TEST-UNEXPECTED-FAIL | browser/components/firefoxview/tests/browser/browser_feature_callout.js | On third screen at a narrower window width, the callout is positioned below the parent element

Looks like these tests only run on backstop pushes (where we run all the tests) and they cannot be triggered manually on try or on other pushes on autoland. Failures in TH on autoland, only linux seems to be affected.

Comment 7

[Norisz Fay [:noriszfay]]

Backed out for causing bc failures on browser_feature_callout.js

Backout link

Push with failures

Failure log

Comment 8

[Meg Viar [:mviar]]

Thanks for your patience as I sort this out. Looks like I'd left some other tests commented out, too.

Unfortunately I don't have a linux machine to test on. I've updated the test case based on a hunch - I'll give it one more go with fingers crossed that fixes it on linux.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/50e2770d7b30
Reposition Feature Callout if it overlaps parent element by more than 15% r=Mardak

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/b1f99e866232
Backed out changeset 1d022fd2c075 for causing bc failures on browser_feature_callout.js CLOSED TREE

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a38c7cbf464
Backed out changeset 50e2770d7b30,1d022fd2c075 for causing bc failures on browser_feature_callout.js CLOSED TREE

Comment 12

[Marian-Vasile Laza]

Backed out changeset 50e2770d7b30,1d022fd2c075 (Bug 1782524) for causing bc failures on browser_feature_callout.js.
Backout link
Push with failures <--> bc1
Failure Log

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12486eec49e2
Backed out changeset 50e2770d7b30 for causing failures on browser_feature_callout.js.

Comment 14

[Natalia Csoregi [:nataliaCs]]

The failure still appeared even after the backout in Comment 12 : https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Ctsan%2Copt%2Cmochitests%2Cwith%2Csoftware%2Cwebrender%2Cenabled%2Ctest-linux1804-64-tsan-qr%2Fopt-mochitest-browser-chrome-swr%2Cbc1&tochange=734f0d5f37479aa0f782829e1ad4d4c65bc15f53&fromchange=55290fbfcb0f57c5098d0cd6a204bca966f660b5

Backed out https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=linux%2C18.04%2Cx64%2Cwebrender%2Ctsan%2Copt%2Cmochitests%2Cwith%2Csoftware%2Cwebrender%2Cenabled%2Ctest-linux1804-64-tsan-qr%2Fopt-mochitest-browser-chrome-swr%2Cbc1&revision=50e2770d7b30d0417b77b5b124dc20f5a91016c6 since the backout in c12 had a weird commit message/backout command and could probably have been rendered null.

Comment 15

[Norisz Fay [:noriszfay]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/12486eec49e2

Comment 16

[Meg Viar [:mviar]]

Thanks, I'll work on debugging the linux failures and plan to land this in 106 nightly.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c02fdd36330b
Reposition Feature Callout if it overlaps parent element by more than 15% r=Mardak

Comment 18

[Cosmin Sabou [:CosminS]]

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

Comment 19

[Simona Rosu [:srosu], Ecosystem QA]

I‘ve verified this enhancement using the latest Firefox Nightly 106.0a1 (Build ID: 20220824213405) on Windows 10 x64, macOS 12.5.1, and Linux Mint 20.2 x64.

• After following the steps from here, I can confirm that:
  • the first screen is displayed below the element with the title "Switch seamlessly between devices";
  • the second screen appears above the recently closed tabs element after clicking the “Next” button from the 1st screen, and it remains in its position when resizing the browser until the Colorways section is displayed above the “Recently closed tabs” section;
  • on a narrow screen, the 3rd screen is displayed above/below the horizontal colorways element, and on wider screens, it is displayed to the left part of the horizontal colorways element.