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

Allow overwrite of window.top #1208

Closed
cpojer opened this issue Aug 21, 2015 · 27 comments
Closed

Allow overwrite of window.top #1208

cpojer opened this issue Aug 21, 2015 · 27 comments
Labels

Comments

@cpojer
Copy link
Contributor

cpojer commented Aug 21, 2015

We have a few tests in jest that attempt to simulate a parent-child relationship between a window and the window of an iframe. With recent versions of jsdom it seems like it isn't possible to overwrite window.top any longer. Are you guys interested in supporting this use case? :)

@domenic
Copy link
Member

domenic commented Aug 21, 2015

You should be able to do so using Object.defineProperty without any issue...

@domenic
Copy link
Member

domenic commented Aug 21, 2015

(Alternately, jsdom's iframe support is pretty solid, so you might be able to use that directly)

@domenic
Copy link
Member

domenic commented Aug 30, 2015

Let me know if anything isn't working with defineProperty! Otherwise, closing for now.

@domenic domenic closed this as completed Aug 30, 2015
@cpojer
Copy link
Contributor Author

cpojer commented Sep 1, 2015

Sorry for taking so long to get back to you, it seems like defineProperty doesn't work in this case, which is why I opened this issue in the first place (although, I did forget whether I tried this before). Unfortunately for now I can't use the iframe support in jsdom – I need my code to run in an iframe-like environment and I'm not ready yet to enable env switching in jest.

I tried this:

      if (!isTop) {
        console.log('top is set before?', !!window.top);
      }
      Object.defineProperty(window, 'top', {
        writable: false,
        value: null,
      });
      if (!isTop) {
        console.log('top is set?', !!window.top);
      }

And window.top is always set and equals window no matter what I use as a value. Using get doesn't work either.

@domenic domenic reopened this Sep 1, 2015
@domenic
Copy link
Member

domenic commented Sep 8, 2015

Investigating. This appears to be an io.js bug with the global/global proxy. Object.getOwnPropertyDescriptor(window, "top") is returning a data descriptor, not an accessor descriptor, which is not good news... :-S

@domenic
Copy link
Member

domenic commented Sep 8, 2015

Commenting out the line

        this._ownerDocument._defaultView._globalProxy = vm.runInContext("this", this._ownerDocument._global);

in level1/core.js "fixes" it, but I have no idea why...

@cpojer
Copy link
Contributor Author

cpojer commented Sep 8, 2015

woah, no idea what is going on here.

@domenic
Copy link
Member

domenic commented Sep 8, 2015

Yeah just kind of giving live updates as I debug... unfortunately this is not as easy as I'd hoped. I'm hoping to find a workaround at the very least though.

@domenic
Copy link
Member

domenic commented Sep 8, 2015

OK, I think I see the issue.

Any "contextified" object, whose getters/setters are handled via the V8 named property callbacks, can only have data properties, it seems. That is, when you ask it what properties it has, it will always tell you it has a data property. Even if we've put a getter on the object.

Chrome gets this wrong too. It has data properties only on window, not getters. That work as actually still under way. And presumably Chrome fixing itself in this way will bring along the relevant fixes to V8.

OK, now about that workaround...

@cpojer
Copy link
Contributor Author

cpojer commented Sep 8, 2015

When you say data property, do you mean a regular property using value in defineProperty or a regular assignment if the property is not further specified?

@domenic
Copy link
Member

domenic commented Sep 8, 2015

Both are the same?

@cpojer
Copy link
Contributor Author

cpojer commented Sep 8, 2015

yes, sorry, I mean whether "data property" is what this kind of value on an object is called.

@domenic
Copy link
Member

domenic commented Sep 8, 2015

By data property I mean any property which reports itself as { value, enumerable, configurable, writable } when queried using Object.getOwnPropertyDescriptor.

@cpojer
Copy link
Contributor Author

cpojer commented Sep 8, 2015

👍 got it.

@domenic
Copy link
Member

domenic commented Sep 8, 2015

OK, the following workaround works. Now I need to figure out how to translate it into something that doesn't use private state and preferably doesn't expose this ridiculous global/global proxy mess to the world:

Object.defineProperty(document._global, "top", {
  get() {
    return "foo";
  }
});

console.log("changed?", window.top === "foo");

document._global is supposed to be never seen by anyone and only used for running scripts. Ugh.

@domenic
Copy link
Member

domenic commented Sep 8, 2015

Filed a Node bug. But it's not looking easy to fix.

The best bet may be for jsdom to stop using getters for these and go back to data properties. I want a day to think it over, but that seems likely to work.

@domenic
Copy link
Member

domenic commented Sep 8, 2015

OKKKKK another awesome factoid: the top property (as well as the window, document, and location properties) are marked [Unforgeable] in the spec. Which means they're supposed to be non-configurable anyway.

So that argues for a jsdom.reconfigureWindow(window, { top, window, document, location }) type API, used only for test-related edge cases like this, which bypasses the Unforgeability from the outside, while still maintaining it while inside the DOM scripts themselves.

There's still the separate problem of top and friends showing up as data descriptors, and not accessor descriptors. But we don't necessarily need to fix that now. Behind the scenes reconfigureWindow can do _global-related hacks for now, and in the future just change the values returned by the getters.

@amelzer
Copy link

amelzer commented Nov 30, 2017

Sorry for digging this out again. Ist there a similar solution for window.parent?

@domenic
Copy link
Member

domenic commented Nov 30, 2017

You can just do window.parent = foo or Object.defineProperty(window, "parent", ...)

@amelzer
Copy link

amelzer commented Nov 30, 2017

That's what I thought, but it doesn't seem to work that way

        window.parent = 'foo';
        console.log(window.parent);
/** prints:
      Window {
        Buffer: 
         { [Function: Buffer]
           poolSize: 8192,
           from: [Function],
           alloc: [Function],
           allocUnsafe: [Function],
           allocUnsafeSlow: [Function],
           isBuffer: [Function: isBuffer],
           compare: [Function: compare],
           isEncoding: [Function],
           concat: [Function],
           byteLength: [Function: byteLength],
           [Symbol(node.isEncoding)]: [Function] },
        process: 
         { title: '/usr/local/bin/node',
           version: 'v8.1.3',
           moduleLoadList: 
...etc
*/

Might be because I trying to do it in jest, though.

@domenic
Copy link
Member

domenic commented Nov 30, 2017

Ah, right, we don't correctly implement [Replaceable]. I'll file another issue for that. In the meantime, Object.defineProperty should work: https://runkit.com/embed/rwmx7ykc0xrq

@Kle0s
Copy link

Kle0s commented Feb 23, 2023

@domenic I actually received TypeError: Cannot redefine property: top when ran your suggestion :/

@ArnaudBuchholz
Copy link

Hello, found this thread while troubleshooting window.top in an iframe. I had to go with window.parent instead, any plan to fix this ?

@AlanSl
Copy link

AlanSl commented Oct 10, 2023

I get TypeError: Cannot redefine property: top if I use Object.defineProperty to redefine top as a property, but this works:

      Object.defineProperty(global, 'top', {
        get() {
          return {}
        },
        set() {
          return {}
        }
      })

The setter is necessary else I get this error:

TypeError: Cannot set property top of [object Window] which has only a getter


It's also possible to simulate being inside a restricted cross-origin iframe (to avoid nasty surprises in production if someone forgot to do window.top !== window.self inside a try-catch) by throwing an error in the getter, like this:

  describe('in iframe', () => {
    const originalWindowTop = window.top as Window
    beforeAll(() => {
      Object.defineProperty(global, 'top', {
        get() {
          throw new Error('Simulating being inside a cross-domain iframe')
        },
        set() {
          return null
        }
      });
    })
    afterAll(() => {
      // Bypass the getter and setter when restoring the property
      Object.defineProperty(global, 'top', originalWindowTop)
    })
    // do some tests

@tinytinystone
Copy link

Hi! I also received TypeError: Cannot redefine property: top error. I tried @AlanSl solution, but it returns same...

@winjeysong
Copy link

I have to downgrade to v20 to overwrite some properties...

refer to: https://github.com/jsdom/jsdom/releases/tag/21.0.0

@winjeysong
Copy link

TypeError: Cannot redefine property

I have to downgrade to v20 to overwrite some properties...

refer to: https://github.com/jsdom/jsdom/releases/tag/21.0.0

After downgrading to v20, new features are not supported and result in new problems... So I have to fork the lastest version of jsdom and publish a new package.
If anyone wants to overwrite built-in properties with jsdom v24, try this: https://www.npmjs.com/package/@new-bit/jsdom

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

No branches or pull requests

8 participants