Skip to content
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

Adds document attribute for disable tcpcheck #4008

Merged
merged 7 commits into from
Nov 5, 2024

Conversation

walmazacn
Copy link
Contributor

@walmazacn walmazacn commented Oct 31, 2024

Description

Changes proposed in this pull request:

  • adds document attribute for disabling 3rd party cookie check
  • removes disableTpcCheck prop to keep one source of truth

Related issue(s)

Resolves #4006

Copy link
Contributor

@JohannesDoberer JohannesDoberer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

export function addInitListener(initFn: (context: Context, origin?: string) => void, disableTpcCheck?: boolean): number;
export type addInitListener = (
initFn: (context: Context, origin?: string) => void,
disableTpcCheck?: boolean
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep the disableTcpCheck argument, it should still be possible to disable via js, e.g. in a test environment.

* @memberof Lifecycle
* @example
* const initListenerId = LuigiClient.addInitListener((context) => storeContextToMF(context))
*/
addInitListener(initFn, disableTpcCheck) {
this.disableTpcCheck = disableTpcCheck;
addInitListener(initFn) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please keep the disableTcpCheck argument, it should still be possible to disable via js, e.g. in a test environment.
if it is true, just call document.head.setAttribute('disable-tpc-check') here instead of this.disableTpcCheck=true

@@ -26,8 +26,8 @@ class LuigiClient {
}
}

addInitListener(initFn, disableTpcCheck) {
return lifecycleManager.addInitListener(initFn, disableTpcCheck);
addInitListener(initFn) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

@@ -140,17 +148,21 @@ class LifecycleManager extends LuigiClientBase {
}

_tpcCheck() {
if (this.currentContext?.internal?.thirdPartyCookieCheck?.disabled || this.disableTpcCheck) {
const tpcCheckDisabled =
this._isTpcCheckDisabled() || this.currentContext?.internal?.thirdPartyCookieCheck?.disabled;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moving the check
this.currentContext?.internal?.thirdPartyCookieCheck?.disabled;
into _isTpcCheckDisabled() could save one LOC ;)

@walmazacn walmazacn merged commit 62c6939 into main Nov 5, 2024
11 checks passed
@walmazacn walmazacn deleted the 4006-document-attribute-for-disable-tcpcheck branch November 5, 2024 08:11
@JohannesDoberer JohannesDoberer added enhancement New feature or request ora ora related issues labels Nov 5, 2024
This was referenced Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ora ora related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Client: use document.head attribute as single-source-of-truth for disable tcpCheck
3 participants