Skip to content

Fix plugin-breaking bug by validating "variable-number" input before saving it#189

Open
SublimePeace wants to merge 1 commit intomgmeyers:mainfrom
SublimePeace:validate-variable-number-input-type-before-save
Open

Fix plugin-breaking bug by validating "variable-number" input before saving it#189
SublimePeace wants to merge 1 commit intomgmeyers:mainfrom
SublimePeace:validate-variable-number-input-type-before-save

Conversation

@SublimePeace
Copy link

Summary
This PR adds input validation before saving values in variable-number type input fields, fixing a critical bug that breaks the plugin, with the only remedy being a complete reinstall of the StyleSettings plugin.

Explanation
The current code allows users to enter and save non-numerical values into the variable-number setting field. When this happens, a NaN value is saved to the CSS variable (due to isFloat ? parseFloat(value) : parseInt(value, 10)), which then turns into null. This is incredibly problematic, as once this happens the user loses the ability to edit this CSS variable, as StyleSettings does not display settings for that variable and any similar variables anymore, as it does not think it's a valid setting at that point.

Reproduction example:

  1. In a new vault, install the Border theme (it works with any theme though), and StyleSettings.

  2. In Style Settings options go to Editor->Headings->Level 1 Headings, and enter any non-numerical value into the H1 font weight setting. Such as asd, a600, hfsdaf, or similar.

25-04-16_d9jVqNfp00

  1. Restart Obsidian

  2. Observe that the setting you just changed is no longer functional. You can type, but it doesn't save anything entered into it anymore, as it's broken. Also, related variable settings are completely gone either.

25-04-16_OYYmlyHPVg

In this example, the following StyleSetting is saved:

{
  "Editor@@h1-weight": null
}

And since this setting makes the h1-weight input field non-functional, along with all related input fields, the plugin is irreversibly broken and can only be fixed with a full reinstall.

@mgmeyers

@SublimePeace
Copy link
Author

Apologies for the double ping @mgmeyers, but according to discord messages people are still getting their Obsidian broken due to this bug, and most importantly they are always unaware of the root cause, and consequently clueless as to how to fix it.

This PR makes minimal changes to a single arrow function in one file, and merging it would resolve this plugin-breaking issue without introducing any bugs, based on my testing.

I know you are busy, so if you require any changes before merging, I'd be happy to make them, just let me know.

Thank you!

@alberti42
Copy link

Is there any follow-up? Why does the author, @mgmeyers, not invite other developers to maintain this great plugin? I can understand he may be very busy. But then he can leave room for others to at least maintain it and resolve incompatibility bugs that may occasionally arise.

Any thoughts how to best proceed?

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.

3 participants