Skip to content

Conversation

@georgewrmarshall
Copy link
Contributor

@georgewrmarshall georgewrmarshall commented Oct 15, 2025

Description

This PR tests whether reverting the LavaMoat patch fixes the popover positioning issue in production builds.

This PR reverts commit ed02ac23e86fc3a20336901a2bef52365cdd6bb1 ("fix: lavamoat patch (#36628)") to test our theory that the LavaMoat policy changes are causing ALL popover components to fail to anchor properly to their reference elements in GitHub Actions builds.

The Problem:

  • All popovers work correctly in local development and 13.5.0 branch builds
  • All popovers fail to position correctly in GitHub Actions production builds on main branch
  • Console errors show "TypeError: e is not a function" indicating @popperjs/core functions are blocked

The Theory:
The LavaMoat policy changes in the reverted commit are preventing @popperjs/core from accessing necessary DOM positioning APIs, causing popovers to float unanchored instead of positioning relative to their anchor elements.

This PR will prove/disprove the theory by:

  1. Reverting the problematic LavaMoat changes
  2. Testing if popovers work correctly in GitHub Actions builds
  3. Determining if this is the root cause

Open in GitHub Codespaces

Changelog

CHANGELOG entry: null

Related issues

Tests fix for popover positioning issue affecting ALL popover components in production builds.

Manual testing steps

  1. Wait for GitHub Actions to build this PR
  2. Download the built extension artifact
  3. Load the extension in browser
  4. Test popover positioning:
    • Click the account menu (top-right)
    • Click the global menu (hamburger icon)
    • Open network selector
    • Verify popovers anchor correctly to their reference elements

Expected Result: If our theory is correct, popovers should position correctly in the GitHub Actions build with the LavaMoat patch reverted.

Screenshots/Recordings

Before (Broken - main branch)

Popovers float unanchored in GitHub Actions builds due to LavaMoat blocking @popperjs/core

After (Testing - this PR)

Should show popovers correctly anchored if LavaMoat revert fixes the issue

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added the team-design-system All issues relating to design system in Extension label Oct 15, 2025
@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

🧩 @MetaMask/extension-devs (1 files, +1 -1)
  • 📁 lavamoat/
    • 📁 build-system/
      • 📄 policy.json +1 -1

📜 @MetaMask/policy-reviewers (1 files, +1 -1)
  • 📁 lavamoat/
    • 📁 build-system/
      • 📄 policy.json +1 -1

Tip

Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers.


🔗 @MetaMask/supply-chain (1 files, +1 -1)
  • 📁 lavamoat/
    • 📁 build-system/
      • 📄 policy.json +1 -1

@georgewrmarshall georgewrmarshall changed the title 🚨 DO NOT MERGE - Document LavaMoat popover positioning issue chore: popover issue with preview builds (DO NOT MERGE) Oct 15, 2025
@github-actions github-actions bot added size-S and removed size-M labels Oct 15, 2025
@georgewrmarshall georgewrmarshall changed the title chore: popover issue with preview builds (DO NOT MERGE) Test: Revert LavaMoat patch to fix popover positioning issue Oct 15, 2025
@georgewrmarshall georgewrmarshall added no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed DO-NOT-MERGE Pull requests that should not be merged labels Oct 15, 2025
@georgewrmarshall georgewrmarshall changed the title Test: Revert LavaMoat patch to fix popover positioning issue chore: popover issue with preview builds Oct 15, 2025
@socket-security
Copy link

socket-security bot commented Oct 15, 2025

@socket-security
Copy link

socket-security bot commented Oct 15, 2025

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring alerts on:

View full report

@georgewrmarshall
Copy link
Contributor Author

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

No policy changes

@georgewrmarshall
Copy link
Contributor Author

@SocketSecurity ignore [email protected]

@georgewrmarshall georgewrmarshall removed the DO-NOT-MERGE Pull requests that should not be merged label Oct 15, 2025
@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: d838472 | Date: 10/15/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.08s (±78ms) 🟡 | historical mean value: 1.05s ⬆️ (historical data)
  • domContentLoaded-> current mean value: 757ms (±73ms) 🟢 | historical mean value: 734ms ⬆️ (historical data)
  • firstContentfulPaint-> current mean value: 82ms (±17ms) 🟢 | historical mean value: 80ms ⬆️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.08s 78ms 1.03s 1.37s 1.35s 1.37s
domContentLoaded 757ms 73ms 714ms 1.04s 991ms 1.04s
firstPaint 82ms 17ms 60ms 204ms 92ms 204ms
firstContentfulPaint 82ms 17ms 60ms 204ms 92ms 204ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [d838472]
UI Startup Metrics (1240 ± 59 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1240110514075912721335
load106595812005410921147
domContentLoaded105895311815310861141
domInteractive18135691742
firstPaint67978121343710891147
backgroundConnect2522392858255271
firstReactRender25185972640
getState14582111727
initialActions40385511
loadScripts81371192151841901
setupStore962431016
WebpackHomeuiStartup8357151141768461036
load63257796478634880
domContentLoaded62457295477624870
domInteractive161157101447
firstPaint17455765167178589
backgroundConnect21104172632
firstReactRender27165883236
getState931631112
initialActions209245
loadScripts62157094374623859
setupStore1052331113
FirefoxBrowserifyHomeuiStartup14291258205013415021692
load1199107314349212731358
domContentLoaded1199107314349212721358
domInteractive993329654104243
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3522148183954
firstReactRender29255453140
getState95689816
initialActions521011149
loadScripts1176105514059012471341
setupStore165247351061
WebpackHomeuiStartup15551344202913116001818
load1331115816249814061527
domContentLoaded1331115816239814051527
domInteractive103303978098350
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect352193134067
firstReactRender322679103440
getState10413415916
initialActions716712341
loadScripts1308114216029513811454
setupStore145251261042
Bundle size diffs [🚀 Bundle size reduced!]
  • background: 58 Bytes (0%)
  • ui: 0 Bytes (0%)
  • common: -111 Bytes (0%)

@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2025
@georgewrmarshall georgewrmarshall deleted the revert-ed02ac2 branch October 17, 2025 19:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

no-changelog no-changelog Indicates no external facing user changes, therefore no changelog documentation needed size-S team-design-system All issues relating to design system in Extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants