POC: Adzerk Fast Fetch with template based preferential render #12765

glevitzky · 2018-01-10T19:21:00Z

Proof of concept in order to determine feasibility of template based AMP creatives for adzerk. AMP cache portions TBD.

AMP ad Fast Fetch implementation for adzerk network allowing for preferential render via mustache template. Template must have previously been pushed to AMP cache (yet to be implemented) in order to guarantee it is A4A valid.

Items to be addressed:

Consult with Adzerk to determine expected publisher tag implementation and resulting ad request url. Currently assumes single attribute named "src" whose format matches: https://adzerk.com\?id=#
Does not yet allow for automatic creation of amp-pixel and amp-analytics from known macro template values
sanitizeHtml strips out all <script> tags within body causing extensions that utilize script for configuration to not work properly (e.g. amp-animations & amp-analytics)
Does not leverage template service. This was intentional design decision but could be altered.
AMP template is "consumed" by the client for preferential render using DOMParser in order to determine fonts/extensions and remove unnecessary portions of head (scripts, boilerplate). This could be moved server side as part of AMP cache integration.
Additional test coverage.

Original PR: #12616

/cc @ampproject/a4a

This reverts commit 2134f8c.

lannka · 2018-01-23T00:21:46Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+}
+
+AMP.extension('amp-ad-template', '0.1', AMP => {
+  AMP.registerElement('amp-ad-template', AmpAdTemplate);


this should not be an extension. it can be just another file in amp-a4a/0.1/ folder for code sharing

lannka · 2018-01-23T06:50:10Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+/** @const {string} */
+const TAG = 'amp-ad-template';
+
+export class AmpAdTemplate {


Naming: AmpAdTemplates since this is more like a template manager class

lannka · 2018-01-23T06:54:16Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+  /**
+   * Fetch and parse template from AMP cache.  Result is stored in global in
+   * order to reduce overhead when template is used multiple times.
+   * @param {number} templateId


as discussed, let's use a templateUrl string.

For example,

Template canonical URL: https://adserver.com/amp_template_1
=>
Template proxy URL https://adserver-com.cdn.ampproject.org/a/s/adserver.com/amp_template_1

The domain name in the proxy URL serves as namespace and the path serves as an ID.
It avoids all the hassles for ad network to submit templates and manage IDs.

lannka · 2018-01-23T06:55:42Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+   * @param {!Element} element Parent element containing template.
+   * @param {!Window=} opt_window
+   */
+  populateTemplate(templateValues, element, opt_window) {


naming nitpick: render()

can we merge element and opt_window?

lannka · 2018-01-23T06:56:06Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+import {dev} from '../../../src/log';
+import {getMode} from '../../../src/mode';
+import {getAmpdoc} from '../../../src/service';
+import {Bind} from '../../amp-bind/0.1/bind-impl';


this will compile the whole amp-bind extension into this extension

How can I avoid doing so?

You might want to get the binder from service.

@choumx correct me if I'm wrong
try: adoptServiceForEmbedIfEmbeddable()

Don't think this is the way. I get the following error:

Error: Service amp-bind not found on parent or doesn't implement EmbeddableService.

Even if this did work, I'd still need some way to call setState().

You should retrieve services from the corresponding function in src/services.js --
in this case, bindForDocOrNull.

lannka · 2018-01-23T06:56:25Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+   * @param {number} templateId
+   * @return {!CachedTemplateDef}
+   */
+  retrieveTemplate(templateId) {


naming nitpick: fetch(templateUrl)

lannka · 2018-01-23T06:58:32Z

extensions/amp-ad-template/0.1/amp-ad-template.js

+    };
+    const cacheKeys = /**@type {!Array<number>}*/
+        (Object.keys(this.templateCache_));
+    if (cacheKeys.length > 5) {


maybe worth to implement a separate helper class: LcuCache?

lannka · 2018-01-23T07:03:17Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+    this.ampCreativeJson_ = null;
+
+    ampAdTemplate = ampAdTemplate ||
+        new AmpAdTemplate(this.win, this.parseTemplate_.bind(this));


instead of passing in parseTemplate function in constructor, can you do:

AmpAdTemplates.fetch(url).then(t => { this.parseTemplate_(t); });

lannka · 2018-01-23T17:47:48Z

extensions/amp-a4a/0.1/amp-ad-templates.js

+
+class LRUCache {
+  /** @param {number} capacity */
+  constructor(capacity) {


you can pass in a generator function in constructor. Then we only need one public method for the LRUCache: get()

Also, worth to extract the code into src/utils/cache.js for sharing.

I'm not really sure how a generator would fit in here. Can you give an example of how you imagine the cache being used? If it's not too strong a preference on your end, I'd prefer to keep it with get/put.

class LRUCache { constructor(capacity, generator); get(id) { ... if (!this.cache_[id]) { this.cache_[id] = generator(id); } return this.cache_[id]; } }

something like this

Sorry, I'm still not understanding. Who calls next(), and how does this eliminate the need for put()?

Since this is holding up release. Let's change this in a future PR. I don't think anyone will be using this code for a while anyway.

it will be used like:

constructor() { this.cache_ = new LRUCache(5, url => { return Services.xhrFor(this.win_).fetchText(url); }); } ... fetch(url) { return this.cache_.get(url); }

there are other more important comments not addressed in this PR, so realistically, i don't think it should hold the release.

Forgot to mark some of the comments as done. Will address the others shortly.

lannka · 2018-01-23T23:30:49Z

There is a couple of remaining comments unaddressed. Please reply to each of them when done.

amphtml-team · 2018-01-24T00:52:12Z

Is the service called amp-bind or bind? I'm not with my laptop. Please check.

…

On Jan 23, 2018 16:39, "glevitzky" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In extensions/amp-ad-template/0.1/amp-ad-template.js <#12765 (comment)>: > + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS-IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {urls} from '../../../src/config'; +import {Services} from '../../../src/services'; +import {dev} from '../../../src/log'; +import {getMode} from '../../../src/mode'; +import {getAmpdoc} from '../../../src/service'; +import {Bind} from '../../amp-bind/0.1/bind-impl'; Don't think this is the way. I get the following error: Error: Service amp-bind not found on parent or doesn't implement EmbeddableService. Even if this did work. I'd still need some way to call setState(). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#12765 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ANd2kEZEUlW0s9btXemvVNfDJoaqYuafks5tNnutgaJpZM4RZ0M3> . -- You received this message because you are subscribed to the Google Groups "amphtml-eng-github" group. To unsubscribe from this group and stop receiving emails from it, send an email to ***@***.*** To post to this group, send email to ***@***.*** To view this discussion on the web visit https://groups.google.com/a/ google.com/d/msgid/amphtml-eng-github/ampproject/amphtml/ pull/12765/review/91032891%40github.com <https://groups.google.com/a/google.com/d/msgid/amphtml-eng-github/ampproject/amphtml/pull/12765/review/91032891%40github.com?utm_medium=email&utm_source=footer> . -- You received this message because you are subscribed to the Google Groups "amphtml-eng" group. To unsubscribe from this group and stop receiving emails from it, send an email to ***@***.*** To post to this group, send email to ***@***.*** To view this discussion on the web visit https://groups.google.com/a/ google.com/d/msgid/amphtml-eng/ampproject/amphtml/pull/ 12765/review/91032891%40github.com <https://groups.google.com/a/google.com/d/msgid/amphtml-eng/ampproject/amphtml/pull/12765/review/91032891%40github.com?utm_medium=email&utm_source=footer> .

amphtml-team · 2018-01-24T00:53:29Z

Yep, you will need setState.

…

On Jan 23, 2018 16:52, "Hongfei Ding (丁鸿飞)" ***@***.***> wrote: Is the service called amp-bind or bind? I'm not with my laptop. Please check. On Jan 23, 2018 16:39, "glevitzky" ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In extensions/amp-ad-template/0.1/amp-ad-template.js > <#12765 (comment)>: > > > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS-IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +import {urls} from '../../../src/config'; > +import {Services} from '../../../src/services'; > +import {dev} from '../../../src/log'; > +import {getMode} from '../../../src/mode'; > +import {getAmpdoc} from '../../../src/service'; > +import {Bind} from '../../amp-bind/0.1/bind-impl'; > > Don't think this is the way. I get the following error: > > Error: Service amp-bind not found on parent or doesn't implement > EmbeddableService. > > Even if this did work. I'd still need some way to call setState(). > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#12765 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/ANd2kEZEUlW0s9btXemvVNfDJoaqYuafks5tNnutgaJpZM4RZ0M3> > . > > -- > You received this message because you are subscribed to the Google Groups > "amphtml-eng-github" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to ***@***.*** > To post to this group, send email to ***@***.*** > To view this discussion on the web visit https://groups.google.com/a/go > ogle.com/d/msgid/amphtml-eng-github/ampproject/amphtml/pull > /12765/review/91032891%40github.com > <https://groups.google.com/a/google.com/d/msgid/amphtml-eng-github/ampproject/amphtml/pull/12765/review/91032891%40github.com?utm_medium=email&utm_source=footer> > . > > -- > You received this message because you are subscribed to the Google Groups > "amphtml-eng" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to ***@***.*** > To post to this group, send email to ***@***.*** > To view this discussion on the web visit https://groups.google.com/a/go > ogle.com/d/msgid/amphtml-eng/ampproject/amphtml/pull/12765/ > review/91032891%40github.com > <https://groups.google.com/a/google.com/d/msgid/amphtml-eng/ampproject/amphtml/pull/12765/review/91032891%40github.com?utm_medium=email&utm_source=footer> > . >

glevitzky · 2018-01-24T01:38:11Z

Doesn't seem to matter. Using 'bind' instead of 'amp-bind', I get `Error: Service bind not found on parent or doesn't implement EmbeddableService.`. On Tue, Jan 23, 2018 at 7:53 PM, amphtml-team <notifications@github.com> wrote:

…

Yep, you will need setState. On Jan 23, 2018 16:52, "Hongfei Ding (丁鸿飞)" ***@***.***> wrote: > Is the service called amp-bind or bind? I'm not with my laptop. Please > check. > > On Jan 23, 2018 16:39, "glevitzky" ***@***.***> wrote: > >> ***@***.**** commented on this pull request. >> ------------------------------ >> >> In extensions/amp-ad-template/0.1/amp-ad-template.js >> <#12765 (comment) >: >> >> > + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS-IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >> + * See the License for the specific language governing permissions and >> + * limitations under the License. >> + */ >> + >> +import {urls} from '../../../src/config'; >> +import {Services} from '../../../src/services'; >> +import {dev} from '../../../src/log'; >> +import {getMode} from '../../../src/mode'; >> +import {getAmpdoc} from '../../../src/service'; >> +import {Bind} from '../../amp-bind/0.1/bind-impl'; >> >> Don't think this is the way. I get the following error: >> >> Error: Service amp-bind not found on parent or doesn't implement >> EmbeddableService. >> >> Even if this did work. I'd still need some way to call setState(). >> >> — >> You are receiving this because you are subscribed to this thread. >> Reply to this email directly, view it on GitHub >> <#12765 (comment) >, >> or mute the thread >> <https://github.com/notifications/unsubscribe-auth/ ANd2kEZEUlW0s9btXemvVNfDJoaqYuafks5tNnutgaJpZM4RZ0M3> >> . >> >> -- >> You received this message because you are subscribed to the Google Groups >> "amphtml-eng-github" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to ***@***.*** >> To post to this group, send email to ***@***.*** >> To view this discussion on the web visit https://groups.google.com/a/go >> ogle.com/d/msgid/amphtml-eng-github/ampproject/amphtml/pull >> /12765/review/91032891%40github.com >> <https://groups.google.com/a/google.com/d/msgid/amphtml- eng-github/ampproject/amphtml/pull/12765/review/91032891% 40github.com?utm_medium=email&utm_source=footer> >> . >> >> -- >> You received this message because you are subscribed to the Google Groups >> "amphtml-eng" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to ***@***.*** >> To post to this group, send email to ***@***.*** >> To view this discussion on the web visit https://groups.google.com/a/go >> ogle.com/d/msgid/amphtml-eng/ampproject/amphtml/pull/12765/ >> review/91032891%40github.com >> <https://groups.google.com/a/google.com/d/msgid/amphtml- eng/ampproject/amphtml/pull/12765/review/91032891% 40github.com?utm_medium=email&utm_source=footer> >> . >> > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#12765 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATkVidBGzwQu317ZzG5l3NmwovAgiUbAks5tNn8TgaJpZM4RZ0M3> .

lannka

Thanks for the changes!

lannka · 2018-01-24T03:19:43Z

examples/adzerk.amp.html

+    <div fallback></div>
+  </amp-ad>
+
+  <!--<amp-ad width=300 height=250


uncomment them?

lannka · 2018-01-24T03:21:52Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+ * amp-mustache.  AMP creative response will consist of the following JSON
+ * object with two fields:
+ *
+ * - ampCreativeTemplateId: number value for template ID.  Template must already


can we follow the naming in the design doc?

templateUrl, data,

lannka · 2018-01-24T03:34:06Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+   */
+  getTemplateProxyUrl_(url) {
+    const loc = parseUrl(url);
+    const hostClean = startsWith(loc.host, 'www.')


no need to remove www..

lannka · 2018-01-24T03:34:11Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+    const hostClean = startsWith(loc.host, 'www.')
+      ? loc.host.slice(4)
+      : loc.host;
+    return loc.protocol + '//' + hostClean.replace('.', '-') + '.' +


please refer to AMP cache URL format

Probably won't fix until morning, but besides removing 'www', what's wrong with this format?

Maybe not clear from the code, but this converts https://adserver.com/amp_template_1 to https://adserver-com.cdn.ampproject.org/a/s/adserver.com/amp_template_1 as mentioned.

Converting the AMP document domain from IDN (Punycode) to UTF-8.

Replacing every "-" (dash) with "--"(2 dashes).

Replacing every "." (dot) with a "-" (dash).

Converting back to IDN (Punycode).

lannka · 2018-01-24T03:43:59Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+ *    string value used to dynamically update the template
+ *
+ * Additionally, ad response must include header indicating AMP creative
+ * template response: AMP-template-amp-creative: true


instead of having the header value to be true, we can put amp-mustache, to differentiate templating languages.

lannka · 2018-01-24T16:38:14Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+   */
+  getTemplateProxyUrl_(url) {
+    const loc = parseUrl(url);
+    return loc.protocol + '//' +


proxy URL is always https

lannka · 2018-01-24T16:38:49Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+      checkStillCurrent();
+      this.ampCreativeJson_ = /** @type {!AmpTemplateCreativeDef} */
+        (tryParseJson(body) || {});
+      const proxyUrl = getMode(this.win).localDev


can you move the proxy URL logic into AmpAdTemplates for sharing?

lannka · 2018-01-24T16:41:35Z

extensions/amp-ad-network-adzerk-impl/0.1/amp-ad-network-adzerk-impl.js

+    const loc = parseUrl(url);
+    return loc.protocol + '//' +
+        loc.hostname.replace(/-/g, '--').replace(/\./g, '-') +
+        '.' + urls.cdn.slice(8) + '/a/s/' + loc.hostname + loc.pathname;


let's use /c/s/ for now, before AMP cache supports /a/s/

lannka

LGTM. good to go, thanks for the work!

lannka · 2018-01-24T17:04:59Z

extensions/amp-a4a/0.1/amp-ad-templates.js

+  /**
+   * Fetch and parse template from AMP cache.  Result is stored in global in
+   * order to reduce overhead when template is used multiple times.
+   * @param {string} templateUrl CDN Proxy URL to template.


canonical URL

lannka · 2018-01-24T17:05:28Z

build-system/app.js

+      `0.1/data/${match[2]}.template`;
+  fs.readFileAsync(filePath).then(file => {
+    res.setHeader('Content-Type', 'application/json');
+    res.setHeader('AMP-template-amp-creative', 'true');


amp-mustache

lannka · 2018-01-24T17:05:38Z

build-system/app.js

+      pc.cwd() + '/extensions/amp-ad-network-adzerk-impl/0.1/data/' + match[1];
+  fs.readFileAsync(filePath).then(file => {
+    res.setHeader('Content-Type', 'application/json');
+    res.setHeader('AMP-template-amp-creative', 'true');


amp-mustache

…oject#12765) * POC: Adzerk Fast Fetch with template based preferential render * fix dep check * fix test failures * Using templating service. * yarn.lock revert * yarn.lock * yarn.lock * Attempt ampproject#4 at yarn fix... * Test fix. * Update mock templates to use <template>s. * Reverting mode-object. * Yarrrrn * Moving toward amp-bind. * amp-bind integration. * Removing the need for amp-state. * Revert "Removing the need for amp-state." This reverts commit 2134f8c. * Modifying transformed template. * Factored our ad templating into new extension. * Deleting test file. * Lint fixes and other minutiea. * Added comments. * PR feedback. * Removed unused function. * Added tests for templates and cache. * Adding tests. * presubmit fixes. * Reverting to amp-mustache. * Url transforming, uncommenting, and other feedback. * Removing duplicate files. * Moving url transform to amp-ad-templates. * Test fixes * header values + minor comment change.

lannka · 2018-01-31T16:06:32Z

For #12903

…oject#12765) * POC: Adzerk Fast Fetch with template based preferential render * fix dep check * fix test failures * Using templating service. * yarn.lock revert * yarn.lock * yarn.lock * Attempt #4 at yarn fix... * Test fix. * Update mock templates to use <template>s. * Reverting mode-object. * Yarrrrn * Moving toward amp-bind. * amp-bind integration. * Removing the need for amp-state. * Revert "Removing the need for amp-state." This reverts commit 2134f8c. * Modifying transformed template. * Factored our ad templating into new extension. * Deleting test file. * Lint fixes and other minutiea. * Added comments. * PR feedback. * Removed unused function. * Added tests for templates and cache. * Adding tests. * presubmit fixes. * Reverting to amp-mustache. * Url transforming, uncommenting, and other feedback. * Removing duplicate files. * Moving url transform to amp-ad-templates. * Test fixes * header values + minor comment change.

glevitzky requested review from keithwrightbos, zhouyx and lannka January 10, 2018 19:21

googlebot added the cla: yes label Jan 10, 2018

glevitzky force-pushed the adzerk_a4a branch from 95b3feb to 6530c40 Compare January 10, 2018 20:13

keithwrightbos and others added 11 commits January 12, 2018 14:38

POC: Adzerk Fast Fetch with template based preferential render

d734655

fix dep check

48e8a15

fix test failures

6691a1e

Using templating service.

f741499

yarn.lock revert

1a5e2b8

yarn.lock

c5ed7d8

yarn.lock

9b155dc

Attempt #4 at yarn fix...

1357e28

Test fix.

55d3e73

Update mock templates to use <template>s.

9482734

Reverting mode-object.

b67d257

glevitzky force-pushed the adzerk_a4a branch from dcaf3c7 to b67d257 Compare January 12, 2018 19:39

glevitzky added 10 commits January 12, 2018 14:43

Yarrrrn

f1e2533

Moving toward amp-bind.

9cdda9a

amp-bind integration.

62aa508

Removing the need for amp-state.

2134f8c

Revert "Removing the need for amp-state."

e183819

This reverts commit 2134f8c.

Modifying transformed template.

2d9483f

Factored our ad templating into new extension.

e8306a5

Deleting test file.

4d20b5a

Lint fixes and other minutiea.

1882831

Added comments.

e9d24fe

lannka reviewed Jan 23, 2018

View reviewed changes

lannka requested changes Jan 23, 2018

View reviewed changes

PR feedback.

08adb9a

Removed unused function.

4b79e19

lannka reviewed Jan 23, 2018

View reviewed changes

glevitzky added 3 commits January 23, 2018 14:38

Added tests for templates and cache.

4d0430a

Adding tests.

600194f

presubmit fixes.

afbac52

Reverting to amp-mustache.

688238b

lannka approved these changes Jan 24, 2018

View reviewed changes

lannka reviewed Jan 24, 2018

View reviewed changes

glevitzky added 2 commits January 24, 2018 00:13

Url transforming, uncommenting, and other feedback.

0f10ef8

Removing duplicate files.

8315820

lannka reviewed Jan 24, 2018

View reviewed changes

glevitzky added 2 commits January 24, 2018 11:44

Moving url transform to amp-ad-templates.

ea8e4d4

Test fixes

93fab6b

lannka approved these changes Jan 24, 2018

View reviewed changes

header values + minor comment change.

4430a8b

lannka merged commit e3f6d9c into master Jan 24, 2018

lannka deleted the adzerk_a4a branch January 24, 2018 20:46

lannka mentioned this pull request Jan 26, 2018

Fix amp-mustache when the template is not attached to DOM yet #12753

Closed

ampprojectbot added the PR use: In Stable label Jan 31, 2018

POC: Adzerk Fast Fetch with template based preferential render #12765

POC: Adzerk Fast Fetch with template based preferential render #12765

Conversation

glevitzky commented Jan 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lannka Jan 23, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glevitzky Jan 24, 2018 • edited

Choose a reason for hiding this comment

dreamofabear Jan 24, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lannka commented Jan 23, 2018

amphtml-team commented Jan 24, 2018 via email

amphtml-team commented Jan 24, 2018 via email

glevitzky commented Jan 24, 2018 via email

lannka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lannka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lannka commented Jan 31, 2018

lannka Jan 23, 2018 •

edited

glevitzky Jan 24, 2018 •

edited

dreamofabear Jan 24, 2018 •

edited