-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fixed issue when clicking the preview button from the node view full template would cause errors due to form token #27
base: master
Are you sure you want to change the base?
Fixed issue when clicking the preview button from the node view full template would cause errors due to form token #27
Conversation
…template would cause errors due to form token
Hm... Are you sure this is the right fix? What if the ezxformtoken extension is installed? As far as I know, that extension rewrites any POST submit to check for Also, I haven't ran into this issue before. Can you detail steps to reproduce this? Thanks! |
I still cannot reproduce this. Clicking the Preview button at the top of the full view takes me to the preview without issues. |
@emodric What is the base ezp + legacy version you are using? |
@thiagocamposviana eZ Platform 2.4.0 and eZ Publish Legacy 2018.09.2 |
Alright, I will try to reproduce and record this sometime next week and I will post the video here. I have just installed eZP 2.4 yesterday and I am sure the form token extension is checking for "_token" instead of "ezxform_token" with legacy bridge. |
Alright, I've reproduced the issue, some caches were probably to blame :-o Anyways, as far as I can see, there are multiple places where |
Looks like a good idea. |
Maybe we should make this configurable and let legacy bridge set ezxformtoken to use the Symfony convention? That is the approach used for more other things around tis. |
@andrerom For Netgen Admin UI, it doesn't need to be configurable, I think, since it cannot be used in pure legacy context. But as an idea it's good for |
This is the context I'm talking about, but yes should have made the comment on the other PR and pinged you ;) |
What do you think @thiagocamposviana ? |
It should be an enhancement, as of right now it as long this is a bug, the present solution solves the bug without allowing any configuration, which is ok to certain extent, the enhancement would be good as a future step but not mandatory atm as long it might take way longer to review and approve and we require a solution for the bug itself not necessary containing the enhancement. |
True, but this one: is not clean. So you kind of need the injection => to handle this cleanly => to be able to get this merged => in order to solve the bug. Right? ;) |
Alright, as long that affects only a php file I think I can make the token identifier configurable easily. |
Fixed issue when clicking the preview button from the node view full template would cause errors due to form token