"Uncaught TypeError: Cannot redefine property: _real_stringify"

[HiEv]

Ran into this weird error when updating this code. When clicking the fullscreen button (happens in Opera and Firefox, though not Chrome) it triggers an "Uncaught TypeError: Cannot redefine property: _real_stringify" error.

I traced the source of the problem down to this code in src/lib/extensions.js:

    Object.defineProperty(JSON, '_real_stringify', {
        value : JSON.stringify
    });

EDIT 2: I thought I verified it with an older version, but I might have not cleared the cache.

[tmedwards]

It's definitely not new code. It's been defined that way since, at least, v2.0.0.

Ran into this weird error when updating this code. When clicking the fullscreen button (happens in Opera and Firefox, though not Chrome) it triggers an "Uncaught TypeError: Cannot redefine property: _real_stringify" error.

I tried that file both online and downloaded locally in various browsers—FF, Edge, Chrome, Opera, Vivaldi, FF (iOS), Safari (iOS)—and could not replicate the issue. I even tried SugarCube's built-in Fullscreen API and no dice—not that I expected there to be.

Based on the error, something attempted to redefine JSON._real_stringify, which cannot happen since it's non-configurable/-writable. Since I doubt that your code is attempting that, I can only guess that it's either a random glitch or, since you say you encountered it in different browsers, some other code—an extension, code you're loading, etc. Even considering the latter possibility, however, this is a bizarre situation since I cannot fathom why anything would be attempting to redefine that static method.

Was there anything at all unusual about how you tested?

[HiEv]

No, nothing unusual about how I tested, unless you count being on Windows 7.

I just tried it out in Brave, Edge, Pale Moon, Vivaldi, CCleaner Browser, and even IE 11, none of which (except for IE) I've even touched the settings in since I only use them for testing, so they're completely default with no added extensions. I just pasted the link I gave above into the URL bar, waited for it to load, clicked the fullscreen button in the UI bar, and (except for CCleaner) they all got the exact same error. In the CCleaner Browser doing that just locks up the tab. Chrome is the only browser that isn't throwing an error for me when I do that.

Are you clicking the fullscreen button on the UI bar?

The odd thing is, the code to do that is (as far as I can tell) exactly the same as the code in my sample code collection, however that code doesn't throw any errors.

Based on the error, something attempted to redefine JSON._real_stringify, which cannot happen since it's non-configurable/-writable.

Perhaps I'm missing something, but isn't that exactly what your code is trying to do?

I dunno. Something weird is going on here.

[tmedwards]

Are you clicking the fullscreen button on the UI bar?

Yes.

Perhaps I'm missing something, but isn't that exactly what your code is trying to do?

That chunk of SugarCube's code is what defines it that way in the first place, at startup and never again. The only way it could result in the error is if your new code was causing your browsers to parse the page again in-place with the existing document and JavaScript instances.

That's not how fullscreen mode works, however, and would likely cause several more issues than just attempting to redefine that one property if it did. Your old code would also result in the error if that were the case, which doesn't seem to be happening, based on what you've said.

I dunno. Something weird is going on here.

Something wicked is definitely about.

[ChapelR]

I was able to reproduce this issue in edge, firefox, and chrome:

[ChapelR]

If you delete this line: <<= _n>> from the widget, everything works. So the problem seems to be with how the generated code from the widget is being stringified (or something), idk, I'm out of time to look at it. My guess is some change in the API is outputting something to the screen that SC can't print/make sense of when the fullscreen occurs. Not sure why exactly it's reporting the error in this way, though, but the issue is definitely related to the values being printed.

[ChapelR]

I spoke too soon, the issue is in printing setup.Fullscreen.fullscreenElement(). You probably want something more like <<PrintType `setup.Fullscreen.fullscreenElement() ? setup.Fullscreen.fullscreenElement().nodeName : setup.Fullscreen.fullscreenElement()`>> (but less sloppy, hopefully). @HiEv

[tmedwards]

@ChapelR Thanks. I haven't had a chance to look at the code yet, but that would definitely cause the reported issue in versions ≥v2.34.0.

@HiEv Since v2.34.0, printing Node instances—incl. Element nodes—will attempt to print a reasonable string representation of the node—see issue #87 for the details. The relevant changelog entry (emphasis added):

• Updated the naked variable markup and <<print>> family of macros to: include default conversions for DOM objects and better conversions for various edge cases values.

In this case, your code is attempting to print the fullscreen element's .outerHTML, which in this case would include SugarCube's main script chunk. Thus, attempting to redefine JSON._real_stringify. Something like Chapel's suggested fix is what you need.

PS: I wasn't able to reproduce the issue initially because the file served by your link at the time I tried was built against v2.31.1—I saved a copy to disk. Now it's serving a file built against v2.34.1 and I can also reproduce the snafu.

[HiEv]

Thanks. I've updated the <<PrintType>> widget as below and that fixed the problem:

<<widget "PrintType">>
    <<set _n = $args[0]>>
    <<if ndef _n>><<set _n = "@@.invert;undefined@@">><</if>>
    <<if _n === null>><<set _n = "@@.invert;null@@">><</if>>
    <<if _n === "">><<set _n = "@@.invert;(empty string)@@">><</if>>
    <<if !!_n && typeof _n === "object">><<set _n = "[Object " + _n.constructor.name + "]">><</if>>
    <<= _n>>
<</widget>>

[HiEv]

It's definitely not new code. It's been defined that way since, at least, v2.0.0.

I forgot to mention that the above is incorrect. That particular code was just added to extensions.js on Jan. 4, 2021 (see this diff).

Also, for anyone wondering what it looked like before I modified the <<PrintType>> widget, here's the earlier version:

<<widget "PrintType">>
    <<set _n = $args[0]>>
    <<if ndef _n>><<set _n = "@@.invert;undefined@@">><</if>>
    <<if _n === null>><<set _n = "@@.invert;null@@">><</if>>
    <<if _n === "">><<set _n = "@@.invert;(empty string)@@">><</if>>
    <<= _n>>
<</widget>>

I added a line to keep it from trying to print (using <<=>>) any objects directly.

[tmedwards]

🤦‍♂️ I suppose that was poor wording on my part. I meant that code exactly like that has been defined that way since v2.0.0.

For example its sibling static method JSON._real_parse is defined exactly the same way and has been since v2.0.0 (specifically since Sep 2015).

The point being that if the problem had actually been the definition of JSON._real_stringify, then JSON._real_parse and similar code would have been exploding since v2.0.0.