Affected versions

• 92.0b2 (20210810185524)
• 93.0a1 (20210810213316)

Affected platforms

• macOS 10.13
• Windows 10x64

Preconditions

• browser.search.region:US

Steps to reproduce

1. Open Firefox and a new tab.
2. Click on the Open Menu from the right side pocket card.

Expected result

• Menu is displayed as expected on the left side.

Actual result

• Menu is cut off and opened on the right side.

Regression range

• Last good revision: ff220475fee8b11104548853a74b021a1718f47e
  First bad revision: 120f83c79d5c22ea8cf344bd901de79333dc035b
  Pushlog:
  https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=ff220475fee8b11104548853a74b021a1718f47e&tochange=120f83c79d5c22ea8cf344bd901de79333dc035b

Notes

• Attached a screenshot.

I suspect the sizing of the popup is not waiting for the UI to be localized and the recent change to l10n microtask exposed this bug.

The way to fix this is to improve the code to wait for DOM Fragment to be localized before sizing it.

:zbraniecki do you have thoughts on how to move this bug forward.

no, I do not know the code in question.

I can take a look today.

The code in question is here.

https://searchfox.org/mozilla-central/source/browser/components/newtab/content-src/components/DiscoveryStreamComponents/DSLinkMenu/DSLinkMenu.jsx#32-36

I think I just need to know when the DOM Fragment is localized. Is there an event or some way to determine that?

I think I just need to know when the DOM Fragment is localized. Is there an event or some way to determine that?

Can you show where the l10n is applied?

I agree that rAF is a perfect way to introduce intermittent bug, so I'm glad we're moving away from it. What it should be replaced with depends on how the l10n binding is constructed.

If I understand you, the l10n binding is happening with data-l10n-id, these menu items get that set here: https://searchfox.org/mozilla-central/source/browser/components/newtab/content-src/components/ContextMenu/ContextMenu.jsx#170

Unfortunately, there is a significant amount of abstraction from the Pocket panel calling rAF to the actual setting of data-l10n-id, that I might need to work around.

Hopefully that helps answer the question.

Ok, that's quite imperfect because what you really would want to do is:

let fragment = document.createDocumentFragment(```
  <div data-l10n-id="key1"/>
```);

await document.l10n.translateFragment(fragment);
root.appendChild(fragment);

this would guarantee that when you inject the fragment it is localized.

In the absence of that, you can just force-translate which will in the worst case re-translate a fragment:

await document.l10n.translateFragment(myFragment);
readTheSizing();

I'll see hat I can do with that tomorrow.

If that doesn't work well, we could also use a mutation observer on the element and resized once the mutation observer fires with a non 0 width?

A hack, but seems pretty straight forward.

In theory one the localization is done it should update the contents of the element, and trigger the mutation observer.

I can't see a reason that forced translation might not work, but a mutation observer would be an acceptable fallback if I'm wrong.

I think I've got a patch for this, with a test to cover this in the future. I put you down as an optional review, feel free to take a look if you want.

The solution to force-translate to re-translate a fragment actually worked pretty well, so went with that.

Comment on attachment 9237104 [details]
Bug 1725160 - Fixing newtab Pocket context menu position.

Beta/Release Uplift Approval Request

• User impact if declined: User experience

• Is this code covered by automated tests?: Yes

• Has the fix been verified in Nightly?: No

• Needs manual test from QE?: Yes

• If yes, steps to reproduce: Steps to reproduce

  Open Firefox and a new tab.
  Click on the Open Menu from the right side pocket card.

Expected result

Menu is displayed as expected on the left side.

Actual result

Menu is cut off and opened on the right side.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a pretty small js change.
• String changes made/needed: none

Backed out for causing failures at browser_newtab_last_LinkMenu.js.

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

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=MXhAEHmrTe2FQ_YjlCDt2w.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=windows%2C10%2Cx86%2Cwebrender%2Cdebug%2Cmochitests%2Ctest-windows10-32-qr%2Fdebug-mochitest-browser-chrome-e10s%2Cbc1&revision=51400ee7a844fb8331e404fb4952d94e54369ab9

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

https://hg.mozilla.org/mozilla-central/rev/26433f71bd48

Comment on attachment 9237104 [details]
Bug 1725160 - Fixing newtab Pocket context menu position.

Thanks for getting this fixed in time. Approved for 92.0b9.

https://hg.mozilla.org/releases/mozilla-beta/rev/91a6eb2958a0

Verified fixed with 93.0a1 (20210829215733) and 92.0b9 (20210826192006) on Windows 10x64, macOS 10.15 and Ubuntu 21.04. The Pocket context menu is opened as expected if triggered from the right side.