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

Jump to bottom

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

Merged
merged 4 commits into from Sep 8, 2015
Merged

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

merged 4 commits into from Sep 8, 2015

Conversation

diosmosis
Copy link
Member

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 diosmosis added c: Privacy For issues that impact or improve the privacy. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. labels Aug 26, 2015
@diosmosis diosmosis added this to the 2.15.0 milestone Aug 26, 2015
@diosmosis
Copy link
Member Author

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

@mattab
Copy link
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 👍

This was referenced Aug 31, 2015
Merged
Closed
Closed
@diosmosis
Copy link
Member Author

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 diosmosis added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Sep 5, 2015
@diosmosis
Copy link
Member Author

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

@tsteur
Copy link
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
Copy link
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
Merge pull request #8646 from piwik/opt_out_no_3rd_party
9df172e 
Always use new window when setting opt-out cookies
@mattab mattab merged commit 9df172e into master Sep 8, 2015
@mattab mattab deleted the opt_out_no_3rd_party branch September 8, 2015 06:40
@mattab mattab changed the title Always use new window when setting opt-out cookies Open a popup in a new window when setting opt-out cookies Sep 8, 2015
@mattab mattab changed the title Open a popup in a new window when setting opt-out cookies Open popup in a new window when setting opt-out cookies Sep 8, 2015
@tsteur
Copy link
Member

tsteur commented Sep 8, 2015

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

@mattab
Copy link
Member

mattab commented Sep 8, 2015

Amazingly, it does work with Javascript disabled :)

@mattab
Copy link
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
Reviewers
No reviews
Assignees
No one assigned
Labels
c: Privacy For issues that impact or improve the privacy. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Needs Review PRs that need a code review
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@diosmosis @mattab @tsteur