Lotame id system domain update #6063
Conversation
…domain; the expiry cookie's expiry to match its value
|
This pull request introduces 1 alert when merging 3447bcd into 6963bb3 - view on LGTM.com new alerts:
|
| findCookieDomain: function (fullDomain) { | ||
| if (!storage.cookiesAreEnabled()) { | ||
| return fullDomain; | ||
| } | ||
|
|
||
| const domainParts = fullDomain.split('.'); | ||
| if (domainParts.length == 2) { | ||
| return fullDomain; | ||
| } | ||
| let rootDomain; | ||
| let continueSearching; | ||
| let startIndex = -2; | ||
| const TEST_COOKIE_NAME = 'lotame_domain_check'; | ||
| const TEST_COOKIE_VALUE = 'writeable'; | ||
| do { | ||
| rootDomain = domainParts.slice(startIndex).join('.'); | ||
| let expirationDate = new Date( | ||
| utils.timestamp() + 10 * 1000 | ||
| ).toUTCString(); | ||
| storage.setCookie(TEST_COOKIE_NAME, TEST_COOKIE_VALUE, expirationDate, 'Lax', `${rootDomain}`, undefined); | ||
| const value = storage.getCookie(TEST_COOKIE_NAME, undefined); | ||
| if (value === TEST_COOKIE_VALUE) { | ||
| continueSearching = false; | ||
| } else { | ||
| startIndex += -1; | ||
| continueSearching = Math.abs(startIndex) <= domainParts.length; | ||
| } | ||
| } while (continueSearching); | ||
| return rootDomain; | ||
| }, |
There was a problem hiding this comment.
there's at least one or two other id modules that are doing essentially the same thing -- finding the root domain at which to write the 1p cookie. what are people's thoughts on moving this to a method in utils or userId module that can be re-used rather than repeated in multiple modules?
cc @patmmccann @Fawke @jdwieland8282 @robertrmartinez @pycnvr
There was a problem hiding this comment.
@smenzer Yes, I like your idea.
There was a problem hiding this comment.
@idettman do you have thoughts here? is this something we can/should break out to be a common userid (or even common prebid global) method?
There was a problem hiding this comment.
Are you all thinking this should be done in context of this PR or separately?
There was a problem hiding this comment.
I think it probably makes the most sense to break this out into a separate PR, then you can use this PR to apply it to the Lotame ID system
There was a problem hiding this comment.
Alright, I will give that shot
There was a problem hiding this comment.
That part is attempted in #6124
There was a problem hiding this comment.
@markaconrad thanks for putting in #6124! I assume once that goes through, you would remove this block and on line 206 below you'd use the user id function instead of this right?
There was a problem hiding this comment.
You are welcome! Yes, that is the plan
|
I'm going to close this out but will re-open with a couple more changes and the result of #6124 |
Type of change
Description of change
Define method on userId example page so the page loads
A few enhancements to the Lotame Panorama Id System User Id module
Be sure to test the integration with your adserver using the Hello World sample page.
Other information