Content.ad: CDN domain processing #7316
Conversation
Converted let variable to const since it is never changed.
|
to @lannka |
|
Ping @lannka |
ads/contentad.js
Outdated
| @@ -35,9 +35,10 @@ export function contentad(global, data) { | |||
| /* Pass Source URL */ | |||
| let sourceUrl = window.context.sourceUrl; | |||
| if (data.url) { | |||
| const host = window.context.location.host; | |||
| const s = document.createElement('a'); | |||
There was a problem hiding this comment.
you can use parseUrl from url.js
There was a problem hiding this comment.
I thought I was using that - see lines 39 and 41 below.
There was a problem hiding this comment.
Are you suggesting that I should use a different method @lannka?
There was a problem hiding this comment.
Yes, you should call parseUrl to replace the anchor element/s.href code here.
There was a problem hiding this comment.
@jridgewell I think I got it. I updated the code to use parseUrl. Please let me know if anything further needs to be done to get this merge request approved.
|
@jridgewell any chance this pull request could go out with this week's release? |
|
We just cut canary last night, so not this week. Additionally, we have a 2 week release schedule, so yours will go into canary next Wed, and production the following Wed. |
|
Oh ok. Previously release day was Thursdays. But next week will have to do. Thank you! |
|
|
LGTM. |
* Content.ad: CDN domain processing * Changed let to const Converted let variable to const since it is never changed. * Updated to use parseUrl method
* Content.ad: CDN domain processing * Changed let to const Converted let variable to const since it is never changed. * Updated to use parseUrl method
Resolve issue where CDN domain processing failed when the host did not match the requesting domain