Change boilerplate bootstrap <style> to <script> #323

dvoytenko · 2015-09-28T15:58:06Z

Milestone: Post Preview

The script is:

  <script>
    (function(w, d) {
      d.documentElement.style.opacity = 0;
      w.onerror = function() {
        d.documentElement.style.opacity = 1;
      };
    })(window, document);
  </script>

And the counterpart in AMP runtime:

    win.document.documentElement.style.opacity = 1;
    win.onerror = ampErrorHandler;

The script itself will live in it's own file.
It will be released on request (not automatically) as following:
2.1. The file will be compiled, minified and obfuscated
2.2. The script will be integrated into validator. Validator will validate against all known versions of this script.
2.3. Once validator is updated, it will be posted in the "Setup" publishers' guide.
We will remove requirement of "amp-custom" in the user stylesheet's <style> element. We will go back to require a single <style> in the document.
We can separately discuss if the proxy should automatically update a matched version of this script to the latest known version.

The text was updated successfully, but these errors were encountered:

adactio · 2015-10-09T00:02:48Z

Looks good. Here's what I proposed in #548...

How about setting the default opacity with JavaScript instead?

<script>document.body.style.opacity=0</script>

It's shorter and has the same effect; if JavaScript isn't available, the CSS doesn't get applied—no need for a noscript element. This way, the opacity is always set with the same technology: JavaScript.

Caveat: I'm guessing that (because of the asynchrous loading of the ampproject JavaScript file), this script element that I'm suggesting (which, like the current style element, should be blocking) would need to be placed before the script element that calls the external file:

<script>document.body.style.opacity=0</script>
<script src="https://cdn.ampproject.org/v0.js" async></script>

@dvoytenko's script looks a lot more robust than my simple suggestion (with the addition of the onerror handler).

In any case, I'm really, really glad to see the opacity reset being moved out of style and into script. And being able to drop the proprietary amp-custom attribute is definitely a bonus!

kzap · 2015-10-09T12:28:22Z

+1 fallback for nonjs or failed js rendering should be part of it, what if the js errors and the body never gets displayed...

dvoytenko · 2015-10-09T17:43:41Z

/cc @Gregable @powdercloud this is coming down the pike.

cramforce · 2015-10-13T03:50:20Z

The primary problem with this is that it does not work with our default CSP.

We cannot use nonces in AMP because they are not compatible with cache-control: public (besides not being supported in Safari).

We could use a hash, but that would very complicated (Maybe OK) but also turn on unsafe-inline in Safari which is an absolute no-go.

So, while I absolutely love the suggestions here I think we cannot implement them until Safari ships a robust CSP implementation. One of our goal in AMP is robust naive serving. Making every single publisher on the planet support a complex dynamic CSP seems very unlikely to be successful.

Does anyone have an idea that works without any inline JS?

dvoytenko · 2015-10-13T04:55:50Z

Yeah, I haven't thought about CSP. It's definitely a bit of a show stopper. But perhaps we can still make our current boilerplate simpler and more reliable? Among other things:

We can always set opacity on documentElement instead of body. Not a huge deal, but a little easier to work with.
Maybe we can figure out a way to remove amp-custom from user style? I think it's better to add an attribute to boilerplate style or maybe none at all - it's kind of clear which is which.
Register default error handler as the first thing in our binary to reset opacity in case of failure.

powdercloud · 2015-10-13T08:48:49Z

I think with the css parser getting ready it should be ok to have the opacity thing inside the same style tag as what's now known as the amp-custom style. We need to play a little bit with this to get a feel for how to implement rules for the css though.

cramforce · 2015-10-13T13:21:17Z

@dvoytenko those are all good suggestions, but they don't address the big win which would be to be robust in the case of JS load failure.

dvoytenko · 2015-10-13T16:00:13Z

@cramforce Yes, unfortunately. But it looks like we don't have much of an option at this time? If we can't inline style and can't do nonce do we have any other option?

One version would have window.onerror handler set up as a first thing not only in the main amp.js binary, but in all extension binaries - these all will reduce the probability of failure without handler. We can also enable viewer integration to do the same.

powdercloud · 2015-10-13T18:34:02Z

Is there some way to listen to the script element that's loading amp.js, like there is with onreadystatechange with xml http requests?
Also, could something where you set a timer be acceptable? E.g., after n seconds of trying to render the doc smoothly, fall back to making whatever is there visible?
(OK, quite possibly these are dumb suggestions feel free to ignore haha)

dvoytenko · 2015-10-13T20:54:49Z

Well, if we can insert inline script - we can do it all with ease as described above. The problem is that our CSP doesn't allow inline styles and things are even more complicated when Safari is involved.

powdercloud · 2015-10-13T22:11:35Z

Abusing a CSS animation is out as well for it, yes?

cramforce · 2015-10-13T22:18:10Z

@powdercloud I think that is an amazing idea!!!

An opacity animation that stays at 0 for N seconds that then goes to 1.

adactio · 2015-10-13T23:12:42Z

@powdercloud @cramforce Ooh, that's smart!

My main concern with the current situation is that mixes JavaScript and CSS, assuming both will work in tandem. A solution that's either all JavaScript or all CSS would be much more robust. Until now, I had really only been thinking of the JavaScript options, but what you're proposing (all CSS) sounds much more straightforward and elegant.

erwinmombay · 2015-10-14T00:13:00Z

@powdercloud hah, that would be a really neat sol'n.

dvoytenko · 2015-10-14T04:05:14Z

Yeah, I think it's a cool idea. We can try that. It could look something like this:

    body {
      opacity: 0;
      animation: amp-starter 0s 10s 1 normal forwards;
    }
    @keyframes amp-starter {
      0%   { opacity: 0; }
      100% { opacity: 1; }
    }

We can give it a max of, say, 10 seconds to wait before the opacity is reset to 1 automatically. We'd still need a JS solution that would reset body at the earliest opportunity to

body {
  opacity: 1;
  animation: none;
}

The benefit here is, of course, that opacity can be reset to 1 if JS fails to load/initialize for any reason. On the other hand we need to consider:

It won't be as critical, but we may still want to use noscript
With vendor prefixes, this CSS will definitely be a copy-paste-only boilerplate

cramforce · 2015-10-14T15:11:25Z

I think a good timeout value would be 3-4 seconds, very similar to Chrome's default timeout for fonts.

adactio · 2015-10-14T15:22:09Z

@dvoytenko wrote:

body {
  opacity: 0;
  animation: amp-starter 0s 10s 1 normal forwards;
}
@keyframes amp-starter {
  0%   { opacity: 0; }
  100% { opacity: 1; }
}

Would it possible to drop the initial opacity: 0 declaration on the body? Right now there's an assumption that both opacity and animation which is a pretty safe assumption, but still a bit nervous-making.

I don't the think the lack of the initial opacity: 0 would cause a flash of content or anything, right? (because the CSS is blocking)

body {
  animation: amp-starter 0s 3s 1 normal forwards;
}
@keyframes amp-starter {
  0%   { opacity: 0; }
  100% { opacity: 1; }
}

dvoytenko · 2015-10-14T15:24:35Z

Ok. I think boilerplate will look like this then:

<style>body{opacity:0;-webkit-animation:-amp-start 0s 10s 1 normal forwards;-moz-animation:-amp-start 0s 10s 1 normal forwards;-ms-animation:-amp-start 0s 10s 1 normal forwards;animation:-amp-start 0s 10s 1 normal forwards}@-webkit-keyframes -amp-start{0%{opacity:0}to{opacity:1}}@-moz-keyframes -amp-start{0%{opacity:0}to{opacity:1}}@-ms-keyframes -amp-start{0%{opacity:0}to{opacity:1}}@keyframes -amp-start{0%{opacity:0}to{opacity:1}}</style>
<noscript><style>body{opacity:1;-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>

It looks like we actually can drop some of prefixes but we'll probably have to keep -webkit :(

A version without opacity:

<style>body{-webkit-animation:-amp-start 0s 10s 1 normal forwards;-moz-animation:-amp-start 0s 10s 1 normal forwards;-ms-animation:-amp-start 0s 10s 1 normal forwards;animation:-amp-start 0s 10s 1 normal forwards}@-webkit-keyframes -amp-start{0%{opacity:0}to{opacity:1}}@-moz-keyframes -amp-start{0%{opacity:0}to{opacity:1}}@-ms-keyframes -amp-start{0%{opacity:0}to{opacity:1}}@keyframes -amp-start{0%{opacity:0}to{opacity:1}}</style>
<noscript><style>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>

This version assumes that zero-offset state in -amp-start animation is executed before the first paint.

cramforce · 2015-10-14T15:26:52Z

@dvoytenko See @adactio's idea that would probably allow being a bit more aggressive on the non-prefix side.

dvoytenko · 2015-10-14T15:28:03Z

@adactio, dropping initial opacity works fine on devices I checked. The animation would have to change to start immediately (currently delayed 10s above) and would run instead for that duration and would need to use a step-end timing function or similar.

I haven't yet found a clear confirmation that the keyframes animation MUST start before the first paint - so that needs to still be confirmed.

dvoytenko · 2015-10-14T15:39:29Z

But I think it's reasonable to expect that zero-offset step is applied immediately. So, I think this should work well.

kevinmarks · 2015-10-14T19:44:06Z

@dvoytenko you have opacity 0 in noscript in #323 (comment)

which is worse than opacity 1 currently; I thought the point was to avoid the noscript distinction entirely

dvoytenko · 2015-10-14T21:30:42Z

@kevinmarks Typo. Fixed. I also added a no-opacity version.

adactio · 2015-10-14T23:29:48Z

@dvoytenko Thanks for testing the version with initial opacity: much appreciated.

rudygalfi · 2016-01-30T21:15:17Z

Closing this assuming #998 covers it and necessary validator changes have happened. Reopen if still needed.

jridgewell · 2016-02-02T22:14:18Z

@sriramkrish85, @cramforce: I believe this can be closed now?

adactio · 2016-02-04T11:16:22Z

The docs are still out of date:

https://www.ampproject.org/docs/get_started/create/basic_markup.html

Can we re-open this until the docs are updated?

powdercloud · 2016-02-05T21:23:21Z

@adactio Thank you I filed ampproject/amp.dev#32.

cramforce · 2016-02-05T21:47:56Z

Next docs push will fix. Should happen any day.

On Fri, Feb 5, 2016 at 1:23 PM, Johannes Henkel notifications@github.com
wrote:

@adactio https://github.com/adactio I filed ampproject/amp.dev#32
ampproject/amp.dev#32.

—
Reply to this email directly or view it on GitHub
#323 (comment).

erwinmombay · 2016-02-05T22:22:49Z

confirming with @Meggin if we can do a release

Meggin · 2016-02-05T22:43:17Z

Need to make a few changes to validation doc. Can you give me an hour?
On Feb 5, 2016 2:23 PM, "erwin mombay" notifications@github.com wrote:

confirming with @Meggin https://github.com/Meggin if we can do a release

—
Reply to this email directly or view it on GitHub
#323 (comment).

erwinmombay · 2016-02-05T23:15:36Z

docs pages should be updated w/in the hour

dvoytenko assigned cramforce Sep 28, 2015

dvoytenko changed the title ~~Change boilerplate bootstrap \<style> to \<script>~~ Sep 28, 2015

dvoytenko mentioned this issue Sep 29, 2015

[WIP] Move point where we make the body visible slightly to the back. #344

Merged

This was referenced Oct 8, 2015

If JS doesn't load (On the train) then page is invisible #518

Closed

Replace CSS opacity reset with JavaScript #548

Closed

erwinmombay added the enhancement label Oct 8, 2015

cramforce mentioned this issue Oct 15, 2015

Why try-catch in v0.js? #631

Closed

dvoytenko assigned camelburrito and unassigned cramforce Nov 11, 2015

camelburrito added the INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code label Nov 12, 2015

camelburrito mentioned this issue Nov 23, 2015

Boilerplate change- Switch from using opacity and settimeout to using visibility and keyframe animation. #998

Merged

rudygalfi closed this as completed Jan 30, 2016

cramforce reopened this Jan 30, 2016

jridgewell mentioned this issue Feb 2, 2016

AMP boilerplate changes to be done after validator changes to suppor… #1067

Merged

cramforce closed this as completed Feb 2, 2016

powdercloud mentioned this issue Feb 5, 2016

https://www.ampproject.org/docs/get_started/create/basic_markup.html should be updated to reflect the new boilerplate ampproject/amp.dev#32

Closed

aidantwoods mentioned this issue Jan 11, 2017

Don't forbid nonces in AMP spec #6992

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change boilerplate bootstrap <style> to <script> #323

Change boilerplate bootstrap <style> to <script> #323

dvoytenko commented Sep 28, 2015

adactio commented Oct 9, 2015

kzap commented Oct 9, 2015

dvoytenko commented Oct 9, 2015

cramforce commented Oct 13, 2015

dvoytenko commented Oct 13, 2015

powdercloud commented Oct 13, 2015

cramforce commented Oct 13, 2015

dvoytenko commented Oct 13, 2015

powdercloud commented Oct 13, 2015

dvoytenko commented Oct 13, 2015

powdercloud commented Oct 13, 2015

cramforce commented Oct 13, 2015

adactio commented Oct 13, 2015

erwinmombay commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

cramforce commented Oct 14, 2015

adactio commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

cramforce commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

kevinmarks commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

adactio commented Oct 14, 2015

rudygalfi commented Jan 30, 2016

jridgewell commented Feb 2, 2016

adactio commented Feb 4, 2016

powdercloud commented Feb 5, 2016

cramforce commented Feb 5, 2016

erwinmombay commented Feb 5, 2016

Meggin commented Feb 5, 2016

erwinmombay commented Feb 5, 2016

Change boilerplate bootstrap <style> to <script> #323

Change boilerplate bootstrap <style> to <script> #323

Comments

dvoytenko commented Sep 28, 2015

adactio commented Oct 9, 2015

kzap commented Oct 9, 2015

dvoytenko commented Oct 9, 2015

cramforce commented Oct 13, 2015

dvoytenko commented Oct 13, 2015

powdercloud commented Oct 13, 2015

cramforce commented Oct 13, 2015

dvoytenko commented Oct 13, 2015

powdercloud commented Oct 13, 2015

dvoytenko commented Oct 13, 2015

powdercloud commented Oct 13, 2015

cramforce commented Oct 13, 2015

adactio commented Oct 13, 2015

erwinmombay commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

cramforce commented Oct 14, 2015

adactio commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

cramforce commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

kevinmarks commented Oct 14, 2015

dvoytenko commented Oct 14, 2015

adactio commented Oct 14, 2015

rudygalfi commented Jan 30, 2016

jridgewell commented Feb 2, 2016

adactio commented Feb 4, 2016

powdercloud commented Feb 5, 2016

cramforce commented Feb 5, 2016

erwinmombay commented Feb 5, 2016

Meggin commented Feb 5, 2016

erwinmombay commented Feb 5, 2016