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

Open popup in a new window when setting opt-out cookies #8646

Merged
merged 4 commits into from Sep 8, 2015

Conversation

Projects
None yet
3 participants
@diosmosis
Member

diosmosis commented Aug 26, 2015

This PR modifies the opt out form to always use a new window when setting the opt out cookies. This should work around all browsers that have issues w/ 3rd party cookies.

Note: This approach will break when some pop-up blockers are used.

Also Note: This PR is not complete.

Refs #8578

TODO

  • Test in IE11.
  • Test in Firefox.
  • Test in chrome.
  • Test in IE7, IE8, IE9.
  • Test in Safari on OS X.
@diosmosis

This comment has been minimized.

Show comment
Hide comment
@diosmosis

diosmosis Aug 30, 2015

Member

Ping @mattab Please give me your thoughts on the pop-blocker issue.

Member

diosmosis commented Aug 30, 2015

Ping @mattab Please give me your thoughts on the pop-blocker issue.

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Aug 31, 2015

Member

Note: This approach will break when some pop-up blockers are used.

if tests work on all vanilla browsers (ie. PR Todo checklist) this is good to go, as it will work on all default configs and so for most people 👍

Member

mattab commented Aug 31, 2015

Note: This approach will break when some pop-up blockers are used.

if tests work on all vanilla browsers (ie. PR Todo checklist) this is good to go, as it will work on all default configs and so for most people 👍

@diosmosis

This comment has been minimized.

Show comment
Hide comment
@diosmosis

diosmosis Sep 4, 2015

Member

Doesn't seem to work on chrome.

EDIT: Works on chrome, but if trusted host is not set for the Piwik URL, it leads to a hard to diagnose error.

Member

diosmosis commented Sep 4, 2015

Doesn't seem to work on chrome.

EDIT: Works on chrome, but if trusted host is not set for the Piwik URL, it leads to a hard to diagnose error.

@diosmosis

This comment has been minimized.

Show comment
Hide comment
@diosmosis

diosmosis Sep 5, 2015

Member

@mattab @tsteur ready for review and/or merge.

Member

diosmosis commented Sep 5, 2015

@mattab @tsteur ready for review and/or merge.

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Sep 7, 2015

Member

Code wise looks okay to me, leaving the rest to @mattab I'm really not much into this topic and not sure re implication of popup blocker etc (which is enabled by default in most browsers nowadays?)

Member

tsteur commented Sep 7, 2015

Code wise looks okay to me, leaving the rest to @mattab I'm really not much into this topic and not sure re implication of popup blocker etc (which is enabled by default in most browsers nowadays?)

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 8, 2015

Member

Looks good to me! 👍 I will merge it and test it on demo2, if there is any problem with popup blockers we could revert this

Member

mattab commented Sep 8, 2015

Looks good to me! 👍 I will merge it and test it on demo2, if there is any problem with popup blockers we could revert this

mattab pushed a commit that referenced this pull request Sep 8, 2015

Matthieu Aubry
Merge pull request #8646 from piwik/opt_out_no_3rd_party
Always use new window when setting opt-out cookies

@mattab mattab merged commit 9df172e into master Sep 8, 2015

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
Scrutinizer No new issues
Details

@mattab mattab deleted the opt_out_no_3rd_party branch Sep 8, 2015

@mattab mattab changed the title from Always use new window when setting opt-out cookies to Open a popup in a new window when setting opt-out cookies Sep 8, 2015

@mattab mattab changed the title from Open a popup in a new window when setting opt-out cookies to Open popup in a new window when setting opt-out cookies Sep 8, 2015

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Sep 8, 2015

Member

Just out of interest... Does it work when JavaScript is disabled? I've never used it

Member

tsteur commented Sep 8, 2015

Just out of interest... Does it work when JavaScript is disabled? I've never used it

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 8, 2015

Member

Amazingly, it does work with Javascript disabled :)

Member

mattab commented Sep 8, 2015

Amazingly, it does work with Javascript disabled :)

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 8, 2015

Member

Btw added the opt-out iframe in this page: http://piwik.org/privacy-policy/ (loading from demo.piwik.org which is running 2.15.0-b5)

Member

mattab commented Sep 8, 2015

Btw added the opt-out iframe in this page: http://piwik.org/privacy-policy/ (loading from demo.piwik.org which is running 2.15.0-b5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment