#7378: Fix page editor detection #7391
Conversation
fregante
force-pushed
the
F/bug/stop-using-isDevToolsPage
branch
from
29daa73 to
03b2427
Compare
| @@ -68,8 +68,8 @@ async function safelyRunBrick( | |||
| } | |||
|
|
|||
| // This must follow the tab existence checks or else it returns false even if the tab simply doesn't exist | |||
| if (!(await canAccessTab(tabId))) { | |||
| throw new BusinessError("PixieBrix doesn't have access to the tab"); | |||
There was a problem hiding this comment.
Unrelated bugfix, but should be straightforward
| isDevToolsPage() || | ||
| // Check for the page editor pagePath in the location pathname | ||
| location.pathname === "/pageEditor.html" | ||
| ) { |
There was a problem hiding this comment.
We've been bitten by this before
| @@ -52,7 +52,7 @@ const startWatching = once(async () => { | |||
| }); | |||
|
|
|||
| export default function useCurrentUrl(): string { | |||
| expectContext("devTools"); | |||
| expectContext("pageEditor"); | |||
There was a problem hiding this comment.
| pageEditor: isPageEditor, | ||
| contentScript: isContentScript, | ||
| sidebar: isBrowserSidebar, | ||
| } as const; |
There was a problem hiding this comment.
It turns out we were making a mistake here. Due to the Map type, all the contexts were specified and allowed by TypeScript, but then fail at runtime.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## feature/7377-uipath-local #7391 +/- ##
==========================================================
Coverage 72.43% 72.43%
==========================================================
Files 1207 1207
Lines 37535 37535
Branches 7053 7051 -2
==========================================================
+ Hits 27187 27188 +1
+ Misses 10348 10347 -1 ☔ View full report in Codecov by Sentry. |
|
Marking @BLoe as primary reviewer |
| @@ -57,14 +57,6 @@ describe("useCurrentOrigin", () => { | |||
| expect(result.current).toBe("chrome-extension://abcxyz/"); | |||
| }); | |||
|
|
|||
| test("if devtools page, should return devtools origin", async () => { | |||
| setContext("devToolsPage"); | |||
There was a problem hiding this comment.
The page editor test is below. We have no React code running on devtools_page
|
No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack. Do not edit this comment manually. |
Note
This PR's
basebranch is #7378What does this PR do?
See issues in:
isDevToolsPagecheck eslint-config-pixiebrix#192Checklist