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

window.location.replace not working - inserts __ into url #3994

Closed
rebeccajulius opened this issue Apr 18, 2019 · 20 comments · Fixed by #5273
Closed

window.location.replace not working - inserts __ into url #3994

rebeccajulius opened this issue Apr 18, 2019 · 20 comments · Fixed by #5273

Comments

@rebeccajulius
Copy link

rebeccajulius commented Apr 18, 2019

Current behavior:

Navigating within our application using location.replace("") in a chained promise handler throws a cross-origin error. In our actual application, we are waiting on a network request, then navigating using location.replace() in a then handler.

I've simplified the code to the most basic thing I can, and am able to reproduce by calling location.replace() in a simple setTimeout.

window.location.replace("") -- works

setTimeout(() =>{
  window.location.replace("");
});

-- gives the following error: SecurityError: Blocked a frame with origin "http://localhost:58236" from accessing a cross-origin frame.

Desired behavior:

I need to be able to call location.replace() in a chained promise handler.

Steps to reproduce: (app code and test code)

https://github.com/beckee/cypress-test-tiny

Versions

Cypress 3.2.0
Mac OSX
Chrome Version 73

@jennifer-shehane
Copy link
Member

If you ever want to work on the window of your application under test, you'll want to use cy.window(), otherwise - you will be trying to access the actual Cypress Test Runner window.

Can you try:

describe('page', () => {
    it('works', () => {
    });

    it('location.replace', () => {
		cy.window().then((win) => {
        	win.location.replace("");
		})
    })

    it('location.replace promise', () => {
		cy.window().then((win) => {
     		setTimeout(() =>{
          		win.location.replace("");
      		});
		})
    })
})

@jennifer-shehane
Copy link
Member

Also - you'll want to visit an actual webpage before attempting to replace the window's location.

@rebeccajulius
Copy link
Author

rebeccajulius commented Apr 19, 2019

I may have simplified this too much then. It's actually my application making the call to window.location.replace. I'll try to update this test ... but the test would actually just click a button in my application. The code in my application behind that button press is what is calling either:

window.location.replace("")

or setTimeout(() => {window.location.replace("")})

@jennifer-shehane
Copy link
Member

Yeah, if you can, update the repo to reflect the application code + tests that reproduce this problem and we can reopen.

@rebeccajulius
Copy link
Author

rebeccajulius commented May 3, 2019

I added some very simple app code that gets closer to showing the problem.

In this most basic example, you can see that clicking Button 1 loads another site within the same site. Clicking Button 2 should do the same within a setTimeout.

If I run the uploaded app code on a simple local server, I will see the test pass, however the URL loaded is not correct. I expect to see identical behavior in clicking both buttons. This may help exhibit the problem enough. If not, I will need to elaborate on this example more next week. Thank you for looking again.

@jennifer-shehane
Copy link
Member

jennifer-shehane commented May 6, 2019

Thanks for providing a reproducible example. I can indeed reproduce this behavior, which should not be happening.

I can also see that the script renders correctly and is not modified by Cypress when rending within Cypress:

Screen Shot 2019-05-06 at 1 40 50 PM

But, when clicking the second button within Cypress, and when the location.replace is called within a setTimeout, the application attempts to navigate to the __/foo/index.html location instead of /foo/index.html as it should.

@cypress-bot cypress-bot bot added the stage: ready for work The issue is reproducible and in scope label May 6, 2019
@jennifer-shehane jennifer-shehane added type: bug stage: ready for work The issue is reproducible and in scope and removed stage: ready for work The issue is reproducible and in scope labels May 6, 2019
@jennifer-shehane jennifer-shehane changed the title location.replace with promise handling window.location.replace not working - inserts __ into url Oct 29, 2019
@beatrichartz
Copy link

For anyone struggling with this in the meantime, a workaround is to reference the window object outside the timeout, and use that reference in the function called by the timeout, e.g:

let windowRef = window
setTimeout(() => {
  windowRef.location.replace('foo/bar.html')
})

@tomtomsen
Copy link

I stumbled across this error today. i used history.replaceState as workaround

history.replaceState(null,'', 'foo/bar.html');

@tomtomsen
Copy link

Workaround by change app code from

setTimeout(() => window.location.href = "#/some/route/1", 500)

to

setTimeout(() => window.location.hash = "#/some/route/1", 500)

Originally posted by @njevdjo in #5731 (comment)

@jbryson3
Copy link

jbryson3 commented Jan 6, 2020

This is an issue for testing our EmberJS app with Cypress. Specifically affecting ember apps that use the hash routing strategy.

@jbryson3
Copy link

@jennifer-shehane Do you have any idea what package is responsible for this behavior? I'd like to work on a fix for it.

@jennifer-shehane
Copy link
Member

We believe this may be similar to (if not the same issue as) #3975 which is a really complex problem being sorted through in this PR. #5273

flotwig added a commit that referenced this issue May 6, 2020
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: ready for work The issue is reproducible and in scope labels May 8, 2020
flotwig added a commit that referenced this issue May 11, 2020
* Add winPropAccessor to security.js, remove other replacers

* Add start of Cypress.resolveWindowReference

* Add regexes for dot and bracket access

* Some security_spec tests pass with new injection

* Add resolveWindowReference unit tests

* Old security_spec now passes with resolveWindowReference

* Inject stub resolveWindowReference so proxy still works outside of Cypress

* wip: rewrite HTML + JS with tokenizer

* Move to using esprima + hyntax to rewrite JS + HTML

* remove comment; oneLine makes the whole thing commented

* Fix tests, apple.com edge case

* wip: add getOrSet

* Revert "wip: add getOrSet"

This reverts commit a5c647c.

* release 3.5.0 [skip ci]

* use recast to replace window property accesses

* replace assignments to top properly

* fix yarn.lock

* bump deps

* update integration tests

* remove old security ts?

* fix integration spec

* always ignore js interception failure

* use globalThis instead of window

* add experimentalSourceRewriting flag

* restore regex-writer spec

* fix types

* update config_spec

* add source rewriting spec

* cleanup

* simplify rewriting logic, move rules into rewriter package

* create threaded rewriting tool for non-streaming use

* update @packages/rewriter to use threads for async

* use async rewriting where convenient

* add worker-shim.js

* add performance info to debug logs

* properly handle +=, -=, ...

* add proxy, rewriter to unit-tests stage

* cleanup

* use parse5 to rewrite HTML, strip SRI

* update tests

* reorganization, cleanup

* rewrite ALL parent, top identifiers except in a few cases

* handle many JS edge cases

* ensure parse5@5.1.1 is installed

* update yarn.lock

* update tests

* add debugging, add tests

* add attempted repro for .href issue

* implement source maps + extending inline source maps

* update opts passing in proxy layer

* fix sourcemap naming structure

* update tests to account for sourcemaps

* sourcemap tests

* remote source maps work

* comment

* update rewriter tests

* clean up TODOs in resolveWindowReference

* remove @types/nock

* clean up todos in deferred-source-map-cache

* fix rewriter build script

* fix concatStream import

* bump expectedresultcount

* clean up js-rules

* threading improvements, workaround for Electron segfault

* no visit_spec for now

* fix 6_visit_spec

* update MAX_WORKER_THREADS

* add repro for #3975

* cleanup

* cleanup

* make better use of namedTypes and builders

* get rid of the horrific closureDetectionTernary

ast-types keeps track of scope, so it is unneeded

* fix #3975, #3994

* add x-sourcemap, sourcemap header support

* snap-shot-it 7.9.3

* add deferred-source-map-cache-spec

* add tests

* Throw error in driver if AST rewriting fails

* Fix "location = 'relative-url'"

* fix max recursion depth

* slim down some fixtures

* fix window.location usage

* don't mess with `frames` at all

* no integration tests

* skip testing apple.com for now

* update wording: regex-based vs. ast-based

* skip real-world tests for now

* add some padding to process.exit workaround

* fix resolvers_spec

* fix html-spec

* cleanup

* Update packages/rewriter/lib/js-rules.ts

* Update packages/driver/src/cypress/resolvers.ts

* just import find by itself

* privatize typedefs for Cypress.state, remove .gitignore, remove dead code

Co-authored-by: Ben Kucera <14625260+Bkucera@users.noreply.github.com>
@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels May 11, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 11, 2020

The code for this is done in cypress-io/cypress#5273, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@laurentenhoor
Copy link

laurentenhoor commented May 20, 2020

In what release and on what date can we expect this bugfix in #5273?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented May 20, 2020

Released in 4.6.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v4.6.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators May 20, 2020
@flotwig
Copy link
Contributor

flotwig commented May 20, 2020

This fix is available starting in 4.6.0 as an experiment which you can access by setting this config option in your cypress.json or elsewhere:

{
	"experimentalSourceRewriting": true
}

The fix is experimental, so there may be some situations where the _/ rerouting is not fixed.

If you're still experiencing _/ rerouting while setting the experimentalSourceRewriting to true in 4.6.0 - open a new issue with a reproducible example + screenshots, etc - filling out our issue template.

@cypress-io cypress-io unlocked this conversation May 20, 2020
@jennifer-shehane jennifer-shehane added type: duplicate This issue or pull request already exists type: bug and removed type: bug type: duplicate This issue or pull request already exists labels May 21, 2020
@laurentenhoor
Copy link

laurentenhoor commented May 21, 2020

This fix is available starting in 4.6.0 as an experiment which you can access by setting this config option in your cypress.json or elsewhere:

{
	"experimentalSourceRewriting": true
}

The fix is experimental, so there may be some situations where the _/ rerouting is not fixed.

If you're still experiencing _/ rerouting while setting the experimentalSourceRewriting to true in 4.6.0 - open a new issue with a reproducible example + screenshots, etc - filling out our issue template.

@flotwig & @jennifer-shehane Thanks a lot, this fix indeed solves the issue with erroneous redirects (__/). However it slows down our tests significantly. A single page VISIT without experimentalSourceRewriting took 3 seconds. After enabling it, a singleVISIT takes 23 seconds. An unacceptable increase for our pipelines. HTTP requests to GET the page's resources seem to be significantly slower.

@jennifer-shehane
Copy link
Member

@laurentenhoor Can you open a new issue demonstrating this? This is definitely not the performance we're expecting and is something we'd like to improve upon. I'd also like to make sure we exclude any other changes from 4.6.0 as the cause.

@laurentenhoor
Copy link

@jennifer-shehane it has been filed here: #7565

@Bharati2411
Copy link

Bharati2411 commented Oct 30, 2020

If i add "experimentalSourceRewriting" : true in cypress.json my page is breaking not seeing expected elements on the page , the page i wanted to redirect without experimentalSourceRewriting in it . I am stuck i can't complete my test without fixing this issue . The page has iframes and those are broken . I am on Chrome

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

Successfully merging a pull request may close this issue.

8 participants