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 Don't try to save blocks in a readonly elemental area #1292

Merged

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Dec 11, 2024

Turns out when the field is read-only we don't have the schema data - but even if we did, we shouldn't try to save data for a read-only field.

Note this fixes two separate issues - please test both of them to ensure the fix works as expected in both scenarios.

Issues

@emteknetnz
Copy link
Member

Tested both scenarios, both are fixed by this PR

@emteknetnz emteknetnz merged commit 4afbc4e into silverstripe:5.3 Dec 11, 2024
26 checks passed
@emteknetnz emteknetnz deleted the pulls/5.3/allow-restore-pages branch December 11, 2024 03:12
@andreaspiening
Copy link

Hi Guy,

this mr fixed an issue I had with the combination of elemental and advanced-workflow, where clicking the approve button failed with a JS error in the onbeforesubmitform hook, due to the form being readonly.

However I wonder if the onmatch hook needs the same treatment: it is triggering the same problem (accessing the the data-schema attribute, which is absent in the readonly form), hence showing an error in the console, though not having any negative impact on submitting of the form.

Andy

@GuySartorelli
Copy link
Member Author

I wonder if the onmatch hook needs the same treatment: it is triggering the same problem (accessing the the data-schema attribute, which is absent in the readonly form), hence showing an error in the console, though not having any negative impact on submitting of the form.

If there's no actual problem other than an error in the console log, then yes technically it is a bug that should be fixed but the priority is too low for me to personally create a PR for it.

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