FIX avoid javascript error when preview is https

[mikenz]

When the admin interface is http but the preview is https, javascript isn't able to read the contentDocument of the secure preview iframe. This currently results in a javascript error. Patch wraps try/catches around the two instances,

[dhensby]

Can we do some kind of console.warn to signify we caught an error - otherwise this is just silently swallowed up.

I appreciate fixing this issue, but how is it coming about that the preview is secure but the CMS isn't? If the site is enforcing SSL (which is the only way I can imagine this is happening) then shouldn't the CMS be https too?

I'll accept this PR with a console.warn added to the catches and also a console.log for the if (!doc) { return; } bit

[mikenz]

We use static publishing in our installation. Once the page has been statically published its viewable on a static domain. CMS is on http://admin.www.example.com (internal access only) pages are statically published to https://www.example.com

I'm not willing to have console.warn and console.log in there as there's nothing to warn anyone about, this will be too noisy for my users. Happy to just keep this in my branch.

[dhensby]

I think that if a user opens the browser console, they probably want to see this kind of thing (and until now they'd have seen exceptions that were thrown instead).

Silently catching exceptions gives devs no way to figure out what is wrong and will make debugging more difficult. I don't think a noisy console for Admin users is a valid reason to deliberately stifle the use of a developer tool

[mikenz]

Currently if a user triggers this scenario the JS stops running and CMS becomes unusable until the page is reloaded.

[tractorcow]

Silently catching exceptions gives devs no way to figure out what is wrong and will make debugging more difficult. I don't think a noisy console for Admin users is a valid reason to deliberately stifle the use of a developer tool

+1 on this.

I think we need the console.warn in this case. If this is really an error condition, should it be console.error instead, though?

[dhensby]

I'm not too fussed about error vs warn.

[dhensby]

merged by hand and added the console.warn as well as built the dist files

[patricknelson]

Just wanted to point out that this affects older versions. Is there a particular reason this was done in master and not in 3? I can submit a quick PR to help ensure we address this bug in older versions as well -- just let me know what branch (here or on the other issue I opened): #5683