search

Lullabot / amp-library

Convert HTML to AMP HTML and report HTML compliance with the AMP HTML specification
Other
382 stars 182 forks source link

Missing 'amp-ad extension script' support #150

Open liluxdev opened 7 years ago

liluxdev commented 7 years ago

Scripts like this are removed if full html document are passed to amp-library:

<script async custom-element="amp-ad" src="https://cdn.ampproject.org/v0/amp-ad-latest.js"></script>

But they shouldn't be stripped...

liluxdev commented 7 years ago

Instead your validator should be updated to generate script dependency as new specs will require it, the official amp validator is saying now:

The tag ‘amp-ad extension .js script’ is missing or incorrect, but required by ‘amp-ad’. This will soon be an error.

ghost commented 7 years ago

The above script tag is also being removed. Any word on when this will be fixed?

dcfairwi commented 7 years ago

I have the same issue, hope to get an update soon.

ghost commented 7 years ago

https://github.com/Lullabot/amp-library/pull/145 << So I suppose this means the amp-ad script tag isn't required, although the validator warns that it is?

ahilles107 commented 7 years ago

It will/can be required soon. Any news about this?

SGudbrandsson commented 7 years ago

@ahilles107 #145 has been merged into the code for 23 days now. @creativeprogramming @andrewdresden @dcfairwi can you confirm that this works for you now?

renzit commented 7 years ago

This isnt work unless you patch with composer because composer download the last tag and that is 1.07 and doenst have the code that was merged in master before the tag was created...

When there be another release in the tags?

SGudbrandsson commented 7 years ago

@renzit I just released a new version, 1.1.0.

https://github.com/Lullabot/amp-library/releases/tag/1.1.0

Can you test the new release?

renzit commented 7 years ago

@sigginet i tested the tag 1.1.0 on stage a few hours ago , and now is in production working 100% OK. I was patching the master but now is clean and this are the fix i check: -The amp ad warning -The implementation of amp-sidebar.

So far so good :+1:
Thnks

tapankumar commented 7 years ago

I checked out the latest code from here and also tried 1.1.2 release however on both codes i am getting below error.

Githubissues.
  • Githubissues is a development platform for aggregating issues.