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

Fix passing state containing regex with backslashes down to the webview #132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rodionovd
Copy link

@rodionovd rodionovd commented Jan 31, 2024

Steps to Reproduce

  1. Open the plugin, enable regex mode.
  2. Using a regex with at least one backslash (e.g. \((\d+)\)), replace something successfully.
  3. Open the plugin again.

Expected Results

You see the same regex and all other settings you've just used on the previous step.

Actual Results

  • The plugin displays a loading screen for a few seconds, then switches to an empty state as if you've never used it before.
  • The "Find in Selection" option is missing even if you, in fact, have something selected on the canvas. (And now "Find in selection" is gone #107)

Explanation

We're currently passing state to the webview by converting it to a JS expression string to be evaluated.

This lead to special characters (e.g. backslashes) not being escaped properly in regular expressions (they need to be escaped twice for this to work) which, in turn, prevents the web app from parsing this JSON string back into an object.

Solution

The solution is to pass the same stringified state but encoded it as base64 so we don't have to worry about escaping special characters and JSON.parse() throwing unhandled exceptions.

Note

I'm attaching a pre-built version of the plugin with this fix applied so anyone can try it and see if it works for them 👇
Find-and-replace.sketchplugin-fix-for-regex.zip

Note

An alternative workaround is to reset the plugin settings, although it won't prevent the issue from re-appearing again the next time you use regular expressions with backslashes to replace something.

We used to pass `state` to the webview by converting it to a JS expression string to be evaluated.

This led to special characters (e.g. backslashes) not being escaped properly in regular expressions (they need to be escaped twice for this to work) which, in turn, prevented the web app from parsing this JSON string back into an object.

The workaround is to pass the same stringified state but encoded it as base64 so we don't have to worry about escaping special characters
@@ -207,9 +207,10 @@ export default function() {
state = Object.assign({}, state, { init })
}
if (isWebviewPresent(windowOptions.identifier)) {
let base64EncodedState = Buffer.from(encodeURIComponent(JSON.stringify(state))).toString("base64")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't figure out how to use window.btoa() in this context so Buffer.toString("base64") to the rescue.

@thierryc
Copy link
Owner

thierryc commented Feb 9, 2024

Hi @rodionovd ,

Thanks for reporting this bug! That will definitely help me fix it faster.

I wasn't able to reproduce the bug myself, but there are a few things that might explain the difference:

  1. Sketch Version: You're right, having different Sketch versions could be the culprit. Could you please let me know the specific version you're using (including any plugins)? I'm currently on version 97 (173164).

  2. System: Are you using Mac 14.3? Sometimes bugs can be specific to mac os version ?

  3. Steps to Reproduce: Could you please outline the exact steps you take to reproduce the bug? The more detailed you are, the easier it will be for me to replicate it.

  4. Additional Information: Are there any other factors you think might be relevant, like file sizes, or other settings?

Once I have this information, I'll try again to reproduce the bug. In the meantime, feel free to share any screenshots, screen recordings, or other files that might be helpful.

Thank you for your cooperation!

@rodionovd
Copy link
Author

rodionovd commented Feb 10, 2024

Ah, my bad: I left the most important bit out of initial report! The thing with a regular expression is that it should contain at least one backslash: e.g. "((\d+))" to match smth like "(123)".

Updated Steps to Reproduce above.

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.

2 participants