NetgenAdminUIBundle icon indicating copy to clipboard operation
NetgenAdminUIBundle copied to clipboard

Fixed issue when clicking the preview button from the node view full template would cause errors due to form token

Open thiagocamposviana opened this issue 6 years ago • 15 comments

Fixed issue when clicking the preview button from the node view full template would cause errors due to form token

thiagocamposviana avatar Dec 20 '18 16:12 thiagocamposviana

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 ezxform_token variable.

Also, I haven't ran into this issue before. Can you detail steps to reproduce this?

Thanks!

emodric avatar Dec 21 '18 09:12 emodric

Tested this in ezp 2.3 after the changes submitted by Philipp to get back the "Preview" button at the top in the node view full view. If you click the button there is going to appear some errors related to the form token and also related to the language.

See:

https://github.com/netgen/NetgenAdminUIBundle/commit/bf0c1ca521f8577199f3bda1cf3968abe1058306

Also: https://github.com/netgen/NetgenAdminUIBundle/commit/48de5858a99167c3da5055963bf101e0c95fd91b

and https://github.com/netgen/NetgenAdminUIBundle/commit/c9a4445f6da86de0aa59decdaa63b43ed6b716d9

thiagocamposviana avatar Dec 21 '18 18:12 thiagocamposviana

I still cannot reproduce this. Clicking the Preview button at the top of the full view takes me to the preview without issues.

emodric avatar Jan 11 '19 11:01 emodric

@emodric What is the base ezp + legacy version you are using?

thiagocamposviana avatar Jan 11 '19 12:01 thiagocamposviana

@thiagocamposviana eZ Platform 2.4.0 and eZ Publish Legacy 2018.09.2

emodric avatar Jan 11 '19 13:01 emodric

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.

thiagocamposviana avatar Jan 11 '19 13:01 thiagocamposviana

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 ezxform_token POST variable is being used in Netgen Admin UI, would you update all of them in this PR, so we can fix them all at once?

emodric avatar Jan 11 '19 15:01 emodric

Looks like a good idea.

thiagocamposviana avatar Jan 11 '19 16:01 thiagocamposviana

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 avatar Mar 22 '19 11:03 andrerom

@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 ezpublish-legacy with legacy bridge.

emodric avatar Mar 22 '19 11:03 emodric

But as an idea it's good for ezpublish-legacy with legacy bridge.

This is the context I'm talking about, but yes should have made the comment on the other PR and pinged you ;)

andrerom avatar Mar 22 '19 11:03 andrerom

What do you think @thiagocamposviana ?

emodric avatar Mar 22 '19 11:03 emodric

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.

thiagocamposviana avatar Mar 28 '19 16:03 thiagocamposviana

True, but this one: https://github.com/ezsystems/ezpublish-legacy/pull/1420

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? ;)

andrerom avatar Mar 29 '19 11:03 andrerom

Alright, as long that affects only a php file I think I can make the token identifier configurable easily.

thiagocamposviana avatar Apr 02 '19 16:04 thiagocamposviana