fix(parse): remove backspace control characters from input to be parsed #53
Conversation
| // so remove that specific one before continuing. | ||
| // SB2 JSONs and SB3 JSONs have different versions of the | ||
| // character serialized (e.g. \u0008 and \b), strip out both versions | ||
| result = JSON.parse(input.replace(/\\b|\\u0008/g, '')); |
There was a problem hiding this comment.
Would this make backspace characters intentionally used as the input to a "key () pressed?" block break, because the input would be empty (and hence wouldn't detect backspace, as it was intended)?
There was a problem hiding this comment.
Yes, this would remove backspace control characters that are intentionally added (via "hacked" arguments).
There was a problem hiding this comment.
In that case, we would be "fixing" those projects in that they would now load, but they would in fact not operate the way they're meant to, correct? I understand that this PR also fixes issues with JP keyboard input, but does that outweigh the half-broken projects this PR would cause?
There was a problem hiding this comment.
Would this PR make a project owner who has an intentional backspace character, when opening their project in the editor view, override their project JSON to remove the backspace character permanently?
I think a fix to this issue is very important, and like the solution presented in this pull request, and I understand that such intentionally 'hacked' projects are a minority, but ideally we'd be able to preserve the functionality of those 'hacked' projects (or at least be able to restore that functionality at a later date).
|
This looks like it will work and fix a lot of projects for which the scripts are hidden! In XML 1.0, there is no good way of representing control characters like BACKSPACE correctly. However, in XML 1.1, (I think) we can use escape sequences like If we're using the former, couldn't we just upgrade to use the latter; and if we're using the latter, the fix should be in the code which emits XML? |
thisandagain
left a comment
thisandagain
left a comment
There was a problem hiding this comment.
Thanks @kchadha and @picklesrus. I know this has taken a lot of work and careful consideration over the past few weeks. Greatly appreciated.
|
@towerofnix, @joker314, thank you for your thoughtful questions! I will try my best to answer some of them below. @towerofnix you asked:
While it's unfortunate that these projects will be broken, projects with this character (whether put their intentionally via hacked blocks or put their unintentionally like the JP keyboard input on MacOS) are currently broken in many ways in Scratch 3. That character is unfortunately not allowed in XML or SVG, which breaks in many places including blocks, the paint editor and the say bubbles. The current state of these projects is unusable and a frustrating experience for many users. The other part of the issue is that the backspace character is not currently supported in the key pressed blocks either (see https://github.com/LLK/scratch-vm/blob/0dffc65ce99307d048f6b9a10b1c31b01ab0133d/src/io/keyboard.js#L46-L65, the backspace key here is ignored in the modifier key portion of the code). So even if we found a way to keep the backspace control character in the project JSON, these projects would still be broken. @joker314, you asked:
That is also an option we explored, but unfortunately couldn't find a way to make it work. XML 1.1 is also a very uncommon version of XML so we were hesitant about replacing a large part of our workflow with that representation, but it's certainly something we considered and explored during the course of our investigation. We also realized that even with a fix where we changed the way scratch-blocks was parsing the XML, there would still be an issue of the same character not being allowed in SVG. |
|
🎉 This PR is included in version 4.3.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Remove backspace control characters that appear anywhere in the project json so that projects like the one referenced in the test fixtures (217845144) and some of the ones mentioned in scratchfoundation/scratch-vm#1077 (e.g. 194603038) have their blocks load correctly.
Backspace control characters seem to be type-able through the default Japanese IME on MacOS (see scratchfoundation/scratch-vm#1873).
Thanks @picklesrus for pairing on this with me!