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

[js]Update * to originHost #19471

Merged
merged 11 commits into from Aug 16, 2022
Merged

[js]Update * to originHost #19471

merged 11 commits into from Aug 16, 2022

Conversation

peterhashair
Copy link
Contributor

@peterhashair peterhashair commented Jul 6, 2022

Description:

Fixes: #19358
Only fix apply issue 1 fixes. update * to originHost. It seems e.origin contain protocol already.

Review

@peterhashair peterhashair marked this pull request as ready for review July 6, 2022 05:47
Peter and others added 2 commits July 11, 2022 10:29
# Conflicts:
#	js/piwik.min.js
#	matomo.js
#	piwik.js
@peterhashair peterhashair added this to the 4.12.0 milestone Jul 13, 2022
@peterhashair peterhashair added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Jul 13, 2022
js/piwik.js Outdated Show resolved Hide resolved
@sgiehl sgiehl removed the Needs Review PRs that need a code review label Jul 14, 2022
update protocol
js/piwik.js Outdated Show resolved Hide resolved
Co-authored-by: Stefan Giehl <stefan@matomo.org>
@peterhashair
Copy link
Contributor Author

build js

@sgiehl
Copy link
Member

sgiehl commented Jul 18, 2022

The code changes are looking ok.
@peterhashair what exactly did you do to test the changes locally? I did not yet have a look for what exactly the changed code is used, would be helpful if you could provide some insights, so I don't need to spent too much time to find some code that allows testing it.

Peter added 2 commits August 1, 2022 10:38
# Conflicts:
#	js/piwik.min.js
#	matomo.js
#	piwik.js
update merge
@justinvelluppillai
Copy link
Contributor

ping @peterhashair this is waiting for your reply to @sgiehl to help him review this effectively.

@peterhashair
Copy link
Contributor Author

peterhashair commented Aug 10, 2022

@sgiehl @justinvelluppillai ops, sorry, I think there is js test for it, which is that one.

test("Test optOut (via iframe)", function () {

But to tests locally. I add this to my

     addEventListener('load', (event) => {
                const targetFrame = window.top.frames[0];
                const targetOrigin = 'https://yourtrackingsite';
                targetFrame.postMessage('{"d":"hello there","maq_initial_value":true}', targetOrigin);
            });

@sgiehl
Copy link
Member

sgiehl commented Aug 12, 2022

@peterhashair the PR contains two added files, that shouldn't exist.

@peterhashair
Copy link
Contributor Author

@sgiehl ops, that was accidentally added, removed.

@sgiehl sgiehl merged commit 2c63f27 into 4.x-dev Aug 16, 2022
@sgiehl sgiehl deleted the m19358 branch August 16, 2022 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JavaScript Security Vulnerability
3 participants