Fix: ReorderHead priorities #580
Conversation
|
@schlessera PTAL |
|
@sebastianbenz Looks good already, but the |
|
@schlessera that's intentional. The viewer JS is only required if the AMP page is served inside a viewer, which is not the case for AMPs transformed by Optimizer. Theoretically, we could leave it in - but I didn't consider it needed when I implemented the transformer. |
|
As I'm running both the tests from the Go version as well as the spec test suite in here, the above was an actual blocker. The viewer JS is not, as the spec test suite doesn't even include it in the first place. So I'll just implement that to get the Go tests to pass and then this is good from my side. |
| @@ -60,12 +60,12 @@ class HeadNodes { | |||
| appendChild(head, this._metaCharset); | |||
| appendChild(head, this._styleAmpRuntime); | |||
| appendChild(head, this._linkStyleAmpRuntime); | |||
There was a problem hiding this comment.
styleAmpRuntime & linkStyleAmpRuntime order needs to be swapped as well.
There was a problem hiding this comment.
the order is irrelevant for these two as they are mutually exclusive (amp-runtime is the inlined version of v0.css).
There was a problem hiding this comment.
Changed the order for consistency and to avoid future confusion.
ReorderHead has different priorities than in the Go version (Fixes #579)
sebastianbenz
force-pushed
the
reorder-head
branch
from
5cf0cb9 to
aaa5476
Compare
sebastianbenz
force-pushed
the
reorder-head
branch
from
aaa5476 to
00959fb
Compare
ReorderHead has different priorities than in the Go version (Fixes #579)