Skip to content

Conversation

MoMannn
Copy link
Contributor

@MoMannn MoMannn commented Oct 13, 2025

Description

Open in GitHub Codespaces

Changelog

CHANGELOG entry:

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

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.

Note

Permissions page now merges sites derived from gator permissions into the list (with metadata and network info) when the revocation feature is enabled, even if no site connection exists.

  • Permissions Page (ui/components/multichain/pages/permissions-page/permissions-page.js):
    • Behavior: When gator permissions revocation is enabled, merges sites that have gator permissions but no site connection into the displayed list; updates total count and rendering to use the merged list.
    • Data assembly:
      • Aggregates unique siteOrigin entries from gator permissions, collecting addresses and first chainId.
      • Resolves networkName via getNetworkNameByChainId and sets subjectType to SubjectType.Website.
      • Obtains name/iconUrl via getTargetSubjectMetadata, falling back to hostname via getURLHostName.
    • Selectors/Imports: Adds getTargetSubjectMetadata, getGatorPermissionsMap, and related utils; switches calculations from sitesConnectionsList to mergedConnectionsList.

Written by Cursor Bugbot for commit 51ea5f2. This will update automatically on new commits. Configure here.

@MoMannn MoMannn self-assigned this Oct 13, 2025
@MoMannn MoMannn requested a review from a team as a code owner October 13, 2025 12:09
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-delegation MetaMask Delegation Team label Oct 13, 2025
if (permission.permissionResponse.address) {
permissionData.addresses.add(
permission.permissionResponse.address.toLowerCase(),
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Undefined Object Access and Type Errors

The getUniqueSiteOriginsFromGatorPermissions function accesses permission.permissionResponse.chainId and permission.permissionResponse.address without ensuring permissionResponse is defined, which can cause TypeErrors. It also calls .toLowerCase() on the address without confirming it's a string, potentially leading to further runtime errors.

Fix in Cursor Fix in Web

});

return mergedConnections;
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Selector Callback Captures External Dependencies

The mergedConnectionsList selector's callback captures sitesConnectionsList and gatorPermissionsMap from its closure. This prevents useSelector from tracking these external dependencies, resulting in stale data when they change. Additionally, the getUniqueSiteOriginsFromGatorPermissions helper function is defined within the component, causing it to be recreated on each render and impacting selector memoization.

Fix in Cursor Fix in Web

@metamaskbot
Copy link
Collaborator

✨ Files requiring CODEOWNER review ✨

👨‍🔧 @MetaMask/core-extension-ux (1 files, +86 -4)
  • 📁 ui/
    • 📁 components/
      • 📁 multichain/
        • 📁 pages/
          • 📁 permissions-page/
            • 📄 permissions-page.js +86 -4

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: 51ea5f2 | Date: 10/13/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.05s (±72ms) 🟡 | historical mean value: 1.05s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 734ms (±69ms) 🟢 | historical mean value: 737ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 77ms (±13ms) 🟢 | historical mean value: 77ms ⬇️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.05s 72ms 1.01s 1.34s 1.26s 1.34s
domContentLoaded 734ms 69ms 697ms 1.01s 932ms 1.01s
firstPaint 77ms 13ms 60ms 180ms 92ms 180ms
firstContentfulPaint 77ms 13ms 60ms 180ms 92ms 180ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [51ea5f2]
UI Startup Metrics (1274 ± 85 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1274112517548513321381
load110197214977411421198
domContentLoaded109496714607211391193
domInteractive19146381742
firstPaint64890149645411071194
backgroundConnect26224151428264275
firstReactRender2516126112435
getState11490101125
initialActions41275517
loadScripts839722100461882941
setupStore95294916
WebpackHomeuiStartup17721515213814418961995
load1445129716619115181611
domContentLoaded1434128816569115051598
domInteractive171169121452
firstPaint2525215543851621399
backgroundConnect311358103652
firstReactRender5019169275792
getState124121121220
initialActions30264314
loadScripts1430128516549115001592
setupStore124119111318
FirefoxBrowserifyHomeuiStartup14521256189513215091738
load1242108814498813171391
domContentLoaded1242108814498813171390
domInteractive1023529052104241
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3120190183447
firstReactRender30245453140
getState839312634
initialActions30315214
loadScripts1220107014208712881366
setupStore13418424763
WebpackHomeuiStartup15071307182211015631756
load1314116715979013821496
domContentLoaded1313116615969013821495
domInteractive95333765695221
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3316197223380
firstReactRender322577103268
getState824411642
initialActions40537315
loadScripts1287115115638513491443
setupStore10411415736

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size-S team-delegation MetaMask Delegation Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants