DevTools: Improve browser extension iframe support #19854

omarsy · 2020-09-17T20:04:54Z

Fixes #18945

Summary

Correct the issue caused by this PR #19827

Test Plan

DevTools: Iframe support

…sue-18945

omarsy · 2020-09-17T20:06:15Z

packages/react-devtools-shared/src/hook.js

+  if (canAccessParentWindow(target) && !isMainWindow(target)) {
+    hook = getMainWindow(target).__REACT_DEVTOOLS_GLOBAL_HOOK__;
+  }
+  if (!hook) {


I handle now the case if we don't have __REACT_DEVTOOLS_GLOBAL_HOOK__ in the parent window

codesandbox-ci · 2020-09-17T20:07:27Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ae0bd28:

Sandbox	Source
React	Configuration

sizebot · 2020-09-17T20:10:31Z

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against ae0bd28

sizebot · 2020-09-17T20:12:36Z

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against ae0bd28

bvaughn

Looks like these changes fix the dev shell regression 👍

So I guess they're good to move forward with.

omarsy · 2020-09-24T10:14:14Z

Thank you @bvaughn !

StanislavMayorov · 2020-10-05T09:11:11Z

@omarsy I've built the extension from master 97625272a, but react devtools doesn't work with iframe. Is it required any extra configuration in my project?
I use different origins for iframe and main app. Web app is started from localhost:3333 and iframe app from localhost:3001.

omarsy · 2020-10-05T09:20:33Z

@StanislavMayorov No you need any extra configuration. Can I see your code ? What kind of iframe do you use (sandbox or not) ?

omarsy · 2020-10-05T09:25:36Z

Your previous devtool work with iframe if you add if you add this code in your iframe

<script type="text/javascript">
      // Enable DevTools to inspect React inside of an <iframe>	
      // This must run before React is loaded	
      __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__;	
    </script>
``` ?

StanislavMayorov · 2020-10-05T09:29:11Z

I don't have a demo that I can share right now. That's sandbox attr.

<iframe
   sandbox="allow-forms allow-same-origin allow-scripts allow-popups"
 />

StanislavMayorov · 2020-10-05T09:29:41Z

Your previous devtool work with iframe if you add if you add this code in your iframe

<script type="text/javascript">
      // Enable DevTools to inspect React inside of an <iframe>	
      // This must run before React is loaded	
      __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__;	
    </script>
``` ?

No, It didn't work for my app.

omarsy · 2020-10-05T09:54:20Z

In the case of the sandbox we cannot always access the parent's window (blocked by the browser for security reasons), that's why it doesn't work. My fix allows this script

<script type="text/javascript">
      // Enable DevTools to inspect React inside of an <iframe>	
      // This must run before React is loaded	
      __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__;	
    </script>

to be done automatically. But this does not solve the problem of the sandbox. Maybe we can solve the case of sandboxed iframes but I think it is necessary to make a big refactoring and at the security level it remains a big question mark.

StanislavMayorov · 2020-10-05T11:02:32Z

I think it doesn't have access to window.parent.__REACT_DEVTOOLS_GLOBAL_HOOK__. But redux dev works fine.

The extension (version 97625272a) crashes If I remove sandbox attribute. Current released version doesn't crash, it just doesn't show components.

StanislavMayorov · 2020-10-07T20:59:27Z

I tried to reproduce the crash using a demo app, but it works okay with different origins. I couldn't find out what is the reason of crash in my app. I tested that sandbox attr doesn't affect the crash in my app.

omarsy · 2020-10-07T21:47:02Z

Perhaps you have an iframe or ( main window) element in your props ? In this case, when we try to dehydrate the props, it causes this issue.
If you add a debbuger in this line you can see which props causes the issue. Thank you for your screen

react/packages/react-devtools-shared/src/utils.js

Line 614 in 4ead6b5

if (data != null && hasOwnProperty.call(data, meta.type)) {

omarsy · 2020-10-08T07:04:24Z

@bvaughn I think it’s not related to my merge. I can reproduce it.
https://codesandbox.io/s/modest-moser-jg9x9?file=/src/index.js
Just need a confirmation @StanislavMayorov to see which prop causes this issue.
I can push a PR for this issue (security crash) if you want

StanislavMayorov · 2020-10-08T08:57:34Z

I don't have crash in this demo.

@bvaughn I think it’s not related to my merge. I can reproduce it.
https://codesandbox.io/s/modest-moser-jg9x9?file=/src/index.js

I'll check props as I know there is no element.

omarsy · 2020-10-08T09:02:39Z

I don't have crash in this demo.

Try to inspect the App

StanislavMayorov · 2020-10-08T11:42:59Z

@omarsy I just open your app and see error, even without react devtools.

But I have following steps in my app:

open app
inspect iframe react component

result: it fails with error.

omarsy · 2020-10-08T12:05:45Z

Yes, normally if it was due to my change it would have crashed before the inspect (because we do several treatments with the window). I think you have a window in one of your props. I think i can have a PR for this issue

bvaughn · 2020-10-08T12:43:54Z

If you are confident that the crash is not related to the previous iframe change, then you can submit a PR that reverts my revert.

StanislavMayorov · 2020-10-08T13:00:11Z

I can check current version with reverted PR.

bvaughn · 2020-10-08T13:08:16Z

@StanislavMayorov You good to build and test this locally? (Let me know if you need me to build and attach a version here.)

omarsy · 2020-10-08T13:15:09Z

With the reverted PR you cannot inspect 'iframe react component' I think. We need to give you a quick fix for the window security crash + iframe support

bvaughn · 2020-10-08T13:16:47Z

Couldn't you add the pass through (like in our fixtures) to test with the reverted DT?

react/fixtures/devtools/regression/16.7.html

Lines 8 to 12 in 6eca8ef

    
           <script type="text/javascript"> 
        
             // Enable DevTools to inspect React inside of an <iframe> 
        
             // This must run before React is loaded 
        
             __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; 
        
           </script>

omarsy · 2020-10-08T14:24:52Z

@bvaughn for this issue https://codesandbox.io/s/modest-moser-jg9x9?file=/src/index.js. Can I submit a PR ?

bvaughn · 2020-10-08T14:42:08Z

Sorry @omarsy but I don't have time to dig into this at the moment. Please feel free to submit a PR if you think it's appropriate :)

StanislavMayorov · 2020-10-10T19:24:24Z

Couldn't you add the pass through (like in our fixtures) to test with the reverted DT?

react/fixtures/devtools/regression/16.7.html

Lines 8 to 12 in 6eca8ef

<script type="text/javascript">

// Enable DevTools to inspect React inside of an <iframe>

// This must run before React is loaded

__REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__;

</script>

Okay, I have this changes in my app. I will test it. Should I use this version?

@StanislavMayorov You good to build and test this locally? (Let me know if you need me to build and attach a version here.)

I have already built it using these steps https://github.com/facebook/react/tree/master/packages/react-devtools-extensions#build-from-source

omarsy · 2020-10-10T19:50:05Z

Okay, I have this changes in my app. I will test it. Should I use this version?

Yes I think you can use it

StanislavMayorov · 2020-10-11T17:22:49Z

I tested this version and added __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; in html.

There's no error on inspector click, but react inspector doesn't work. It throw only an error on start: Blocked a frame with origin "http://localhost:3002" from accessing a cross-origin frame. on script in html.
Inspector works fine if I test with disabled CORS in chrome.

omarsy · 2020-10-11T17:28:39Z

__REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; react inspector should work if you add this.

StanislavMayorov · 2020-10-11T17:30:23Z

__REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; react inspector should work if you add this.

Iframe app doesn't have access to parent.__REACT_DEVTOOLS_GLOBAL_HOOK__;

I use different origins for iframe and main app. Web app is started from localhost:3333 and iframe app from localhost:3001.

omarsy · 2020-10-11T17:46:32Z

What you say :
this __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; causes crash in start and with my version it causes a crash when you try to inspect the iframe ?

omarsy · 2020-10-11T18:26:48Z

So I think this PR is the reason of crash.

What I'm saying is in the stacktrace, you can see that we have the issue when trying to desyhdrate props. So my understanding is that you should have a window element in your props because of this but maybe I'm wrong that's why I asked you to put in a debugger to see which props were causing the issue If you add a debbuger in this line you can see which props causes the issue. .

Well I'll stop investigating as I can see I'm not getting anywhere.

StanislavMayorov · 2020-10-11T19:52:46Z

What you say :
this __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__; causes crash in start and with my version it causes a crash when you try to inspect the iframe ?

You are right. There are two errors. I missed the error on start in console because react-error-overlay doesn't work at this stage. I update my comment about the test result.

The fix adds script __REACT_DEVTOOLS_GLOBAL_HOOK__ = parent.__REACT_DEVTOOLS_GLOBAL_HOOK__;, but it causes error in case of different origins, doesn't it? That's not good for default behavior.

StanislavMayorov · 2020-10-11T19:53:25Z

I will try to reproduce and debug props soon. Is it enough just to use build:dev?

bvaughn · 2020-10-12T12:08:38Z

Is it enough just to use build:dev?

That's fine. If you're testing in Chrome, the fastest way would be to run either "build:chrome" or "build:chrome:dev", e.g.

yarn build:chrome && yarn test:chrome --url=<url-of-your-sandbox>

Co-authored-by: Joel DSouza <joel.dsouza@kapturecrm.com> Co-authored-by: Damien Maillard <damien.maillard@dailymotion.com> Co-authored-by: Brian Vaughn <brian.david.vaughn@gmail.com>

…19854)" (facebook#19959) This reverts commit a99bf5c.

Joel DSouza and others added 13 commits September 6, 2020 14:13

DevTools: Iframe support

16f07ad

DevTools: Iframe support

Add all_frames:true to chrome devtool extension

0ed8e93

Add "all_frames": true to edge extension manifest.json

988c450

Add "all_frames": true to firefox manifest.json

2575e2f

Move iframe files to fixtures directory

4de1f9d

Make iframe server file executable

1e38987

devtools support iframe

05f64d9

Merge remote-tracking branch 'upstream/master' into issue-18945

0cc8ade

change var to targetWindow

0dd15fc

Added a log to test server to help me remember the port

554ea7e

Merge remote-tracking branch 'upstream/master' into issue-18945

6d34c72

Merge branch 'issue-18945' of https://github.com/omarsy/react into is…

c25f2be

…sue-18945

handle case: parent window have no __REACT_DEVTOOLS_GLOBAL_HOOK__

acb21b5

facebook-github-bot added the CLA Signed label Sep 17, 2020

omarsy commented Sep 17, 2020

View reviewed changes

delete unused function

ae0bd28

bvaughn self-assigned this Sep 23, 2020

bvaughn approved these changes Sep 23, 2020

View reviewed changes

bvaughn merged commit a99bf5c into facebook:master Sep 23, 2020

omarsy deleted the issue-18945 branch October 11, 2020 18:28

omarsy mentioned this pull request Oct 13, 2020

Fix: ReactDevTool crashes if we have sandbox iframe in props #19994

Closed

koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021

Revert "DevTools: Improve browser extension iframe support (facebook#…

9cebb19

…19854)" (facebook#19959) This reverts commit a99bf5c.

awells111 added a commit to awells111/react that referenced this pull request Jul 25, 2022

Committed changes from facebook#19854

5918001

awells111 mentioned this pull request Jul 26, 2022

DevTools: Improve browser extension iframe support #18945

Closed

DevTools: Improve browser extension iframe support #19854

DevTools: Improve browser extension iframe support #19854

Conversation

omarsy commented Sep 17, 2020 • edited Loading

Summary

Test Plan

omarsy Sep 17, 2020 • edited Loading

Choose a reason for hiding this comment

codesandbox-ci bot commented Sep 17, 2020 • edited Loading

sizebot commented Sep 17, 2020 • edited Loading

Size changes (experimental)

sizebot commented Sep 17, 2020 • edited Loading

Size changes (stable)

bvaughn left a comment

Choose a reason for hiding this comment

omarsy commented Sep 24, 2020

StanislavMayorov commented Oct 5, 2020

omarsy commented Oct 5, 2020

omarsy commented Oct 5, 2020

StanislavMayorov commented Oct 5, 2020

StanislavMayorov commented Oct 5, 2020

omarsy commented Oct 5, 2020

StanislavMayorov commented Oct 5, 2020 • edited Loading

StanislavMayorov commented Oct 7, 2020

omarsy commented Oct 7, 2020 • edited Loading

omarsy commented Oct 8, 2020 • edited Loading

StanislavMayorov commented Oct 8, 2020

omarsy commented Oct 8, 2020 • edited Loading

StanislavMayorov commented Oct 8, 2020 • edited Loading

omarsy commented Oct 8, 2020

bvaughn commented Oct 8, 2020

StanislavMayorov commented Oct 8, 2020

bvaughn commented Oct 8, 2020

omarsy commented Oct 8, 2020

bvaughn commented Oct 8, 2020 • edited Loading

omarsy commented Oct 8, 2020 • edited Loading

bvaughn commented Oct 8, 2020

StanislavMayorov commented Oct 10, 2020

omarsy commented Oct 10, 2020 • edited Loading

StanislavMayorov commented Oct 11, 2020 • edited Loading

omarsy commented Oct 11, 2020

StanislavMayorov commented Oct 11, 2020 • edited Loading

omarsy commented Oct 11, 2020 • edited Loading

omarsy commented Oct 11, 2020

StanislavMayorov commented Oct 11, 2020 • edited Loading

StanislavMayorov commented Oct 11, 2020

bvaughn commented Oct 12, 2020

omarsy commented Sep 17, 2020 •

edited

Loading

omarsy Sep 17, 2020 •

edited

Loading

codesandbox-ci bot commented Sep 17, 2020 •

edited

Loading

sizebot commented Sep 17, 2020 •

edited

Loading

sizebot commented Sep 17, 2020 •

edited

Loading

StanislavMayorov commented Oct 5, 2020 •

edited

Loading

omarsy commented Oct 7, 2020 •

edited

Loading

omarsy commented Oct 8, 2020 •

edited

Loading

omarsy commented Oct 8, 2020 •

edited

Loading

StanislavMayorov commented Oct 8, 2020 •

edited

Loading

bvaughn commented Oct 8, 2020 •

edited

Loading

omarsy commented Oct 8, 2020 •

edited

Loading

omarsy commented Oct 10, 2020 •

edited

Loading

StanislavMayorov commented Oct 11, 2020 •

edited

Loading

StanislavMayorov commented Oct 11, 2020 •

edited

Loading

omarsy commented Oct 11, 2020 •

edited

Loading

StanislavMayorov commented Oct 11, 2020 •

edited

Loading