New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates expected Adzerk URL format. #13106
Conversation
examples/adzerk.amp.html
Outdated
@@ -14,21 +14,21 @@ | |||
|
|||
<amp-ad width=300 height=250 | |||
type="adzerk" | |||
src="https://adzerk.com?id=0"> | |||
src="https://engine.adzerk.net/amp?id=0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you might not want the URL to be from the pub, we can hard code in the extension.
also, the id
param is only for dev purpose.
} | ||
// TODO(adzerk): specify expected src path. | ||
return /^https:\/\/adzerk.com\?id=\d+$/i.test(src) ? src : ''; | ||
return `https://engine.adzerk.net/amp?r='${data}'`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should URL encode the data
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually encode the single quotes as well
encodeURIComponent(`'${data}'`)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} | ||
// TODO(adzerk): specify expected src path. | ||
return /^https:\/\/adzerk.com\?id=\d+$/i.test(src) ? src : ''; | ||
return `https://engine.adzerk.net/amp?r='${encodeURIComponent(data)}'`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you need to encode the single quotes as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
it('should be invalid', () => { | ||
try { | ||
impl.getAdUrl(); | ||
expect(false).to.be.true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no try-catch needed if you do
expect(() => imp.getAdUrl()).to.throw(/Expected data-r attribte/);
For #12903 |
* Updated expected Adzerk URL format. * Moving away from using src attr on amp-ad. * Using data-r. * Test update. * Removing unused src attr from test. * Encoding json object. * Encoding quotes. * Test fix * Removing external quotes and added one more test.
* Updated expected Adzerk URL format. * Moving away from using src attr on amp-ad. * Using data-r. * Test update. * Removing unused src attr from test. * Encoding json object. * Encoding quotes. * Test fix * Removing external quotes and added one more test.
* Updated expected Adzerk URL format. * Moving away from using src attr on amp-ad. * Using data-r. * Test update. * Removing unused src attr from test. * Encoding json object. * Encoding quotes. * Test fix * Removing external quotes and added one more test.
* Updated expected Adzerk URL format. * Moving away from using src attr on amp-ad. * Using data-r. * Test update. * Removing unused src attr from test. * Encoding json object. * Encoding quotes. * Test fix * Removing external quotes and added one more test.
* Updated expected Adzerk URL format. * Moving away from using src attr on amp-ad. * Using data-r. * Test update. * Removing unused src attr from test. * Encoding json object. * Encoding quotes. * Test fix * Removing external quotes and added one more test.
Ensures that ad requests can only be sent to:
https://engine.adzerk.net/amp
. Note that query params may be appended to the end of the URL; this is especially the case for local testing, which relies on some appended id to differentiate between different templates in the /data/ directory.