Rely on DOMContentLoaded instead of onload

[ryanseddon]

onload event will delay if downloading external resources when really all we need to know is if the initialContent has rendered without worry about external resources being downloaded

Related to #206

[calculuschild]

@ryanseddon Sorry, this took a while for me to test.

Unfortunately this still doesn't work. I get the same issue where handleLoad() never fires because everything seems to be already loaded before the event listener is added. I added console logs all over frame.jsx and I see the event handler get loaded but handleLoad never triggers from anywhere.

I have set up a staging site for my page where you can see the issue happening. Hopefully this helps debug:

https://homebrewery-stage.herokuapp.com/

The react-frame-component is in the panel on the right half of the page. If you navigate around the site to different pages, create a document, then go to some other page (google.com), then use the back button to return, you will sometimes get a page that just has a perpetual "loading circle" over a dark blue background which is my placeholder until the frame fires contentDidMount. Sometimes just clicking a nav button on the top (NEW, SHARE, etc.) will cause the same failure on the page it loads. It is very inconsistent but forcing a hard refresh on the different pages also seems to trigger the issue when navigating back and forward.

(p.s., in componentWillUnmount you are still clearing the load listener even though it's not added anymore).

Edit: Perhaps also related: https://stackoverflow.com/questions/39993676/code-inside-domcontentloaded-event-not-working

It would seem even with DOMContentLoaded it is possible to miss the timing on the event listener in react components. They suggest something like this:

if( document.readyState !== 'loading' ) {
    console.log( 'document is already ready, just execute code here' );
    myInitCode();
} else {
    document.addEventListener('DOMContentLoaded', function () {
        console.log( 'document was not ready, place code here' );
        myInitCode();
    });
}

function myInitCode() {}

[ryanseddon]

So I tried something along those lines of checking readyState but found that even when chromium had a readyState of complete it would still throw as the mount target wasn't actually ready, I had to reintroduce the onLoad attribute to the iframe otherwise safari wouldn't render at all. Let me publish another alpha and you can try that

[calculuschild]

Sure thing. Were you able to see the bug on the staging site I linked?

[ryanseddon]

Just pushed v5.2.2-alpha.1.

Were you able to see the bug on the staging site I linked?

Is it just that the preview doesn't render? What's the steps to reproduce?

[calculuschild]

The preview should render the markdown from the left panel into the right panel, yes. If the right panel is only showing a blue background then you have encountered the bug; investigating the element will show the iframe is empty. Reproducing it isn't an exact thing, but in general the bug seems most likely to appear after one of the following:

• On the first load of the staging site
• After navigating to another site and then returning via the Back button
• Performing a hard refresh or clearing the cache and then refreshing the page

Although sometimes just simply navigating around the site, anything where the iframe is created fresh (loading a new "document", going forward or back in the browser history, etc.) can trigger the bug intermittently.

[baptisteArno]

Everything works for me on v5.2.2-alpha.1. (It didn't load on Safari with v5.2.2-alpha.0)

[ryanseddon]

Yep had to reintroduce the onLoad attribute otherwise Safari never rendered anything. Thanks for testing.

[calculuschild]

@ryanseddon In case you missed my last comment, I unfortunately still have the same problem, even on v5.2.2-alpha.1 . The DOMContentLoaded event fires before componentDidMount, so the event handler is not present. onLoad doesn't seem to be firing at all. (latest version of Chrome)

This is pretty easy to replicate by running in Chrome Incognito Mode so nothing is cached when you first open the page. If you haven't been able to replicate it there give Incognito a try. It's still up at https://homebrewery-stage.herokuapp.com/ where I have v5.2.2-alpha.1 installed from a couple weeks ago. Just open on a fresh Incognito window. It may take a minute for the server to wake up (It's on a free tier so it sleeps when not in use), which can bypass the issue, but once it's up, just close and reopen Chrome Incognito and visit again. It will fail to fire handleLoad.

[baptisteArno]

Ok I figured my use case only works if the <Frame> component is conditionnaly rendered.

@calculuschild Here is a workaround:

export const MyPage = () => {
  const [show, setShow] = useState(false);

  useEffect(() => {
    setShow(true);
  }, []);

  return <>{show && <Frame .../>}</>;
};

[calculuschild]

@baptisteArno Do you know why it only works for you when it's rendered conditionally? That seems like react-frame-component is still broken if it requires a workaround like this.

[baptisteArno]

Yes it's surely due to server side rendering. Because it won't trigger the useEffect on the server, the frame will only be displayed on the client. But I'm no expert

[ryanseddon]

Ok I can finally recreate the issue on your provided url, thanks.

Documenting some of my findings so I don't lose context of this.

Steps to reproduce:

1. If the url is loaded for the first time it'll fail to call the DOMContentLoaded event listener as the iframes readyState is complete
2. Subsequent reloads will work fine as the resources are coming from browser cache
3. With devtools open click and hold the refresh button to select "Empty Cache and hard reload" will fail to run
4. You can also select the "Disable cache" checkbox in the devtools network cache to always get the failed result

So when the browser has a cold start to this url something gets the timing all messed up as the DOMContentLoaded event is always attached but the readyState is complete when it fails and interactive when it works, but locally just checking whether the readyState is complete isn't sufficient as it's always complete on the example src code in the repo.

Possible solution is to have a timeout to check the this.state.iframeLoaded after sometime and see if its false still or postMessage.

@calculuschild are you able to try adding a message listener to you parent page:

window.addEventListener('message', ({data}) => console.log('data from postMesage: ', data)

ANd then adjust your inititialContent prop to have an onload attribute on the body tag:

props.intitialContent = `<!DOCTYPE html><html><head>
	<link href="//use.fontawesome.com/releases/v5.15.1/css/all.css" rel="stylesheet" />
	<link href="//fonts.googleapis.com/css?family=Open+Sans:400,300,600,700" rel="stylesheet" type="text/css" />
	<link href='/homebrew/bundle.css' rel='stylesheet' />
	<base target=_blank>
	</head><body onload="parent.postMessage('iframe loaded', '*')" style='overflow: hidden'><div></div></body></html>

<body onload="parent.postMessage('iframe loaded', '*')" style='overflow: hidden'><div></div></body>

I'm fairly confident this would be a potential avenue to explore when the timing is off due to cold caches.

[calculuschild]

@ryanseddon I added the message listener to our page now. I see the iframe loaded message being sent on a success, and don't see the iframe loaded message when the iframe fails. Care to give it a look?

[calculuschild]

@ryanseddon Just curious if you've had a chance to look around our staging app after I added your requested message listeners. Does that help you pinpoint the issue so we can fix this? I'm happy to keep it online for you until this is solved, but I can't guarantee it will be available forever.

[ryanseddon]

@calculuschild this is a weird one that I'm having trouble getting to the bottom of. I set up mitmproxy to modify some of the response bodies and if I do that then the issue cannot be recreated so i'm a little stuck.

Unsure where to go from here.

[ryanseddon]

@calculuschild I just published v5.2.3-alpha.0.

I used chrome overrides to modify your bundle.js to patch in the change and it works on cold cache load! Can you confirm that that is the case for you too?