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

"The options are written in an incorrect format." when saving Trumbowyg settings #15703

Closed
Piedone opened this issue Apr 9, 2024 · 5 comments · Fixed by #15768
Closed

"The options are written in an incorrect format." when saving Trumbowyg settings #15703

Piedone opened this issue Apr 9, 2024 · 5 comments · Fixed by #15768
Labels
Milestone

Comments

@Piedone
Copy link
Member

Piedone commented Apr 9, 2024

Describe the bug

When saving the Trumbowyg settings, at least for Html Field, you get the "The options are written in an incorrect format." error with the default options.

To Reproduce

  1. Set up a fresh site from main with the Blog recipe.
  2. Go to /Admin/ContentTypes/Edit/RawHtml.
  3. Edit the Content Field.
  4. What type of editor should be used? -> Trumbowyg editor.
  5. Change nothing, Save.
  6. See the "The options are written in an incorrect format." validation error.

Expected behavior

The default options just work.

Screenshots

2024-04-09_17h59_03.mp4
@Piedone Piedone added this to the 1.9 milestone Apr 9, 2024
@Piedone
Copy link
Member Author

Piedone commented Apr 9, 2024

This is a regression since 1.8.2.

@Piedone
Copy link
Member Author

Piedone commented Apr 9, 2024

@MikeAlhayek this looks like a JSON issue too.

@Skrypt
Copy link
Contributor

Skrypt commented Apr 9, 2024

That's a good example of update regression. 😄

@MikeAlhayek
Copy link
Member

I am guessing that Json contains a trailing comma or contains comments.

@Piedone
Copy link
Member Author

Piedone commented Apr 10, 2024

The problem is that since 1ee808f the Trumbowyg editor's settings (and since then, a lot of other code using IsJson()) is validated as JSON. However, those settings aren't, and never were, actually JSON. Since this is not JSON, but otherwise a valid JS object:

{
    autogrow: true,
    btns: [
        ["viewHTML"],
        ["undo", "redo"],
        ["formatting"],
        ["strong", "em", "del"],
        ["foreColor", "backColor"],
        ["superscript", "subscript"],
        ["link"],
        ["insertShortcode"],
        ["image"],
        ["align"],
        ["unorderedList", "orderedList"],
        ["horizontalRule"],
        ["removeformat"],
        ["fullscreen"]
    ],
    btnsDef: {
        align: {
            dropdown: ["justifyLeft", "justifyCenter", "justifyRight", "justifyFull"]
            ico: "justifyLeft"
        },
        image: {
            dropdown: ["insertImage", "base64", "noembed"],
            ico: "insertImage"
        }
    }
}

We need to keep this for Trumbowyg.

So, for these, maybe we need JS, not JSON validation instead? Either Jint Engine.PrepareScript(), or going one lower, what it does with new JavaScriptParser().ParseScript() (we mustn't try to execute the script, of course), and similarly catching the exception on error.

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

Successfully merging a pull request may close this issue.

3 participants