Migrating multiple fixes upstream #164
Conversation
The DOM parser throws away invalid HTML. When you have http://youtube.com/embed/_FlV6pgwlrk&list=123 then the `IframeYouTubeTagTransformPass::getYouTubeCode()` will match the ID as `_FlV6pgwlrk&list=123` but since the ampersand isn't in a proper HTML format (`&`), then DOM will throw this whole element away. I wrote a test to make sure noone breaks this in the future.
SGudbrandsson
force-pushed
the
upstream-migration
branch
from
9c57892
to
a4122b6
Compare
| @@ -174,6 +174,17 @@ public function loadHtml($html, $options = []) | |||
| $options['use_html5_parser'] = true; | |||
| } | |||
|
|
|||
| // By default the convertion of img into amp-anim is disabled (because of ressource cost) | |||
There was a problem hiding this comment.
What do you mean by resource costs?
There was a problem hiding this comment.
If the amp-anim is enabled, all gif and png images are downloaded by the server and processed bytewise to detect if they are animated or not.
If they are animated, the amp-anim tag is added.
If they are not animated, a normal amp-img tag is added.
This can consume a lot of resources if you don't cache the AMP output in your system.
There was a problem hiding this comment.
It might make sense to document this somewhere (we probably should add the config options to the readme).
There was a problem hiding this comment.
Alright .. I created a new issue with a checklist of what needs to be done here: #168
| /** | ||
| * Identifies APNGs | ||
| * Written by Coda, functionified by Foone/Popcorn Mariachi#!9i78bPeIxI | ||
| * This code is in the public domain |
There was a problem hiding this comment.
Can you add a link to the existing code?
There was a problem hiding this comment.
Well spotted.
I've added the links to the existing code.
SGudbrandsson
force-pushed
the
upstream-migration
branch
from
20a7a9d
to
0e89046
Compare
<amp-img>when image height is missing