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

[WIP]Add special url flag for the authentication via social networks in iframe #1539

Closed
wants to merge 1 commit into from

Conversation

LavrovArtem
Copy link
Contributor

related with #1428

@testcafe-build-bot
Copy link
Collaborator

❌ Tests for the commit 5c75186 have failed. See details:

@LavrovArtem
Copy link
Contributor Author

This PR is unnecassary because the x-frame-options header can be replaced via the Request Hooks feature.

@gbatz
Copy link

gbatz commented May 11, 2019

Hi @LavrovArtem
Could you please share how exactly can the x-frame-options header be replaced via Request Hooks?
I implemented the hook, it fires up for my specific request, but still that header seems to be unaffected.
I'm using your workaround for Facebook Login popup. Setting the 'x-frame-options': skip, in header-transforms works good, but I don't want to keep locally that change in node_modules.
Thanks!

@LavrovArtem
Copy link
Contributor Author

Hi @gbatz,
For your scenario, you should use the _onConfigureResponse method. Unfortunately, it is not documented. The event object has the setHeader (name, value) and removeHeader (name) methods. For example:

class MyRequestHook extends RequestHook {
    constructor (requestFilterRules, responseEventConfigureOpts) {
        super(requestFilterRules, responseEventConfigureOpts);
    }
    onRequest (event) {
    }
    onResponse (event) {
    }
    _onConfigureResponse (event) {
        event.removeHeader('x-frame-options');
    }
}

@gbatz
Copy link

gbatz commented May 13, 2019

Thanks @LavrovArtem for your quick response! That worked like a charm!
I can only hope, still, that new window popups will be supported by testcafe in the near future. There's still lots of other pain with this.
Thanks for your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants