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

{{post::}} insert tag is not working in 4.6 #58

Closed
benfolds opened this issue Sep 13, 2018 · 75 comments
Closed

{{post::}} insert tag is not working in 4.6 #58

benfolds opened this issue Sep 13, 2018 · 75 comments
Assignees
Labels
Milestone

Comments

@benfolds
Copy link

Affected version(s)
contao 4.6.x

Description
The {{post::}} insert tag will not be replaced.

How to reproduce
Make a new form, with a form field named "test", then use {{post::test}} to print the content. (checked at demo.contao.org)
This should be the default behaviour.
contao/core-bundle#1230

Maybe related to
contao/core-bundle#1550

@leofeyer
Copy link
Member

@contao/developers Probably caused by our $_SESSION['FORM_DATA'] changes?

@aschempp
Copy link
Member

🤦‍♂️ most likely …

@leofeyer leofeyer added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Sep 17, 2018
@leofeyer
Copy link
Member

The insert tag should work on the redirect target page of the form (but not on any subsequent page). Can you confirm this? If not, please describe more detailed how to reproduce this.

@leofeyer leofeyer removed the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Sep 27, 2018
@asaage
Copy link

asaage commented Sep 27, 2018

I like the change that the insert-tag is only replaced on the redirect-target-page.
In contao/core-bundle#1230 (comment) you stated its not doable because of multipage forms.
Can one of the pilots tell if contao-mp_forms is still working then?

@Toflar
Copy link
Member

Toflar commented Sep 27, 2018

Can one of the pilots tell if contao-mp_forms is still working then?

It is, mp_forms manages its own session.

@padarom
Copy link

padarom commented Oct 1, 2018

Steps to reproduce:

  1. Create a new form (redirect page in my case was Home)
  2. Create a form field (name test)
  3. Add the form to a page (Home in my case)
  4. Add a new content element with type Text and content Form: {{post:test}}
  5. Write something in the form field
  6. The content element should output Form: <input>, but in my case only outputs Form:

I tested this on https://demo.contao.org/

@Seefahrer
Copy link

Can confirm that e.g. {{post::name}} is also not working on the redirect-target-page.

@frontendschlampe
Copy link
Contributor

We have the same problem!

@leofeyer
Copy link
Member

The problem is that we no longer access $_SESSION['FORM_DATA'] if there is no session. I have no idea how to fix this though. @contao/developers /cc

@leofeyer leofeyer added bug up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. labels Oct 22, 2018
@ausi
Copy link
Member

ausi commented Oct 23, 2018

How can there something be stored in $_SESSION['FORM_DATA'] if there is no session?

Maybe we are using the wrong check currently?
IMO we should check if there is a session for the current request (i.e. a session cookie is present and matches a session stored on the server).

@aschempp
Copy link
Member

The problem is that we no longer access $_SESSION['FORM_DATA'] if there is no session.

We're not checking if there is a session, but whether the user has a previous session. Which basically means if a session cookie is present.
https://github.com/contao/contao/blob/master/core-bundle/src/Resources/contao/library/Contao/Input.php#L781

@ausi
Copy link
Member

ausi commented Nov 22, 2018

We could store the post data as usual in the session and add a response listener which clears the data from the session. This way {{post::}} should be available on the jumpTo page.

@leofeyer leofeyer assigned Toflar and unassigned ausi Nov 22, 2018
@leofeyer leofeyer removed the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Nov 22, 2018
@leofeyer
Copy link
Member

As discussed in Mumble on November 22nd, this is a bug that needs to be fixed. @ausi and @Toflar know how (or at least they know where to start looking).

@Toflar
Copy link
Member

Toflar commented Nov 23, 2018

I did know where to start looking but I did not find a way to fix it. There are actually multiple issues here. One is that the ClearFormDataListener clears the form data during replay-header preflight requests. This is something I could actually fix. However, that would not fix the issue either because {{post::*}} insert tags are converted to ESI requests to work around the page cache. Now if you think about having an external proxy such as Varnish or LiteSpeed it would mean that these are all separate requests. Depending on whether you had a cache entry before or not, the first {{post::*}} would work or not even the first one would because the main response would trigger unsetting the form data. Actually we would need to know which one of the {{post::*}} ESI requests is the last one on the same page and only then we can clear the data. And there's no way to determine that. I'm sorry, I don't see how this can be fixed.

@leofeyer leofeyer added the up for discussion Issues and PRs which will be discussed in our monthly Mumble calls. label Nov 23, 2018
@aschempp
Copy link
Member

aschempp commented Nov 26, 2018 via email

@Toflar
Copy link
Member

Toflar commented Nov 27, 2018

No that's not possible. I've tried that but the master response is generated before the ESI responses. Otherwise, how would you know what ESI tags are present? :) So at that point, it's already too late.

@ausi
Copy link
Member

ausi commented Nov 27, 2018

How about rendering the {{post::}} tag not as ESI but inline and disable the cache if {{post::}} ist used?

With #204 (comment) the {{post::}} tag should get obsolete and we could deprecate it IMO.

@fritzmg
Copy link
Contributor

fritzmg commented Nov 28, 2018

I don't think the {{post::}} insert tag will get obsolete with those changes. You might still want to show data from the form you just submitted in the contextual text field.

@Toflar
Copy link
Member

Toflar commented Nov 28, 2018

I have another idea which is not ideal but should work: We might add a $SESSION['FORM_DATA']['TIME'] = time(); to the data and delete it only after e.g. 30 seconds. That should be way enough for confirmation pages and it will disable caching only for a very short amount of time. During that time also existing cache entries are being invalidated (as long as we have header-replay) so we have to make sure it's as short as possible. 30 seconds sounds okay to me but we might even lower it to 10 seconds because we're only talking about the time needed from submitting the form to the confirmation page. Also, for maximum compatibility for those that do funny stuff like foreach((array) $SESSION['FORM_DATA']... we might just store it as $SESSION['FORM_DATA_LAST_SUBMITTED_AT'] or something similar.

@fritzmg
Copy link
Contributor

fritzmg commented Nov 28, 2018

Hm, 10 seconds might not be enough if an SMTP server is involved (though I don't know which times count). At least I had some cases where the SMTP server took very long to actually process the outgoing email and respond.

@baslerbikes
Copy link

baslerbikes commented Mar 25, 2019

Nachdem ich jetzt nochmal alle Rocksolid Anwendung eine nach der anderen gelöscht habe und keine nennenswerte Veränderung der Zeit, bis die Weiterleitungsseite angezeigt wird, feststellen konnte, immer knapp über 10 sec, blieb nur noch das Notification Center als "Fremdlösung" übrig.
Notification Center gelöscht, Übertragung bei knapp über 3 sec. ... nur ohne NotificationCenter ist meine Website nur die Hälfte wert :-(
Alle Rocksolid Erweiterung die ich brauche wieder installiert ... Weiterleitungsseite erscheint nach 3 sec.

@baslerbikes
Copy link

Lade ich das NotificationCenter neu bin ich wieder bei knapp über 10sec bis die Weiterleitungsseite erscheint

@Toflar
Copy link
Member

Toflar commented Mar 25, 2019

Was hast du denn eingestellt? Als Gateway? Einen eigenen SMTP-Server?

@baslerbikes
Copy link

Standard E-Mail-Gateway
"SMTP- Einstellungen überschreiben" NICHT ausgewählt

@Toflar
Copy link
Member

Toflar commented Mar 25, 2019

Ich glaube dein Server hängt einfach ewig beim Mailversand.

@baslerbikes
Copy link

Danke für Deine schnelle Reaktion!
Hetzner hat das ja schon überprüft. Was schlägst du vor. Was kann ich tun?

Wir haben nun Ihr Skript getestet und zeitgleich auf die Datenbank geschaut, um zu prüfen, welche Verbindungen offen sind. Scheinbar mach Ihr System auch während der Ladezeit eine Verbindung auf, diese befindet sich aber zumeist im Sleep-Modus, es wird also zum Großteil der Zeit keine Anfrage an den SQL-Server geschickt, die Verbindung zum Server jedoch offen gehalten. Das spricht dafür, dass Sie die MySQL-Verbindung beim Start der Routine direkt öffnen, Ihr Skript dann aber sehr lange braucht, um Daten zu senden. Um sicher zu gehen, haben wir noch die genannte Datenbank mittels "mysqlcheck" geprüft - hier war alles in Ordnung. Wir vermuten daher das Problem innerhalb der Programmierung des Formular-Plugins für Contao. Bitte wenden Sie sich hier an den Entwickler des Plugins, da nur dieser genau weiß, was im Hintergrund bei der Verwendung des Plugins passiert.

@fritzmg
Copy link
Contributor

fritzmg commented Mar 25, 2019

Dauert das Abschicken des Formulares auch ohne Notification Center lange, wenn du die Email über die reguläre Contao Funktionalität verschicken lässt?

@baslerbikes
Copy link

baslerbikes commented Mar 25, 2019

wenn ich z.B. einen Newsletter verschicke dauert es in der Tat auch fast 10sec bis der raus ist

Wie ich das Formular ohne NFC sende ist mir nicht klar

@fritzmg
Copy link
Contributor

fritzmg commented Mar 25, 2019

Naja - teste es zuerst mal mit dem Formular ;)

@baslerbikes
Copy link

baslerbikes commented Mar 25, 2019

Ich verstehe nicht wirklich was ich machen soll?
Ich sende das Formular mit "Anmelde" Button ab.
Habe ich das NFC installiert dauert es knapp über 10sec.
Ist das NFC gelöscht dauert es knapp über 3sec.
Sehe ich gerade den Wald vor Bäumen nicht ... ?
Bin, an dieser Stelle leider, Fahrradprofi nicht Websiteprofi ...

Kann ich das NFC auch irgendwo "abschalten", ohne zu löschen?

@fritzmg
Copy link
Contributor

fritzmg commented Mar 25, 2019

Ich verstehe nicht wirklich was ich machen soll?

Die Contao-eigene Funktionalität zum Versenden der Formulardaten via Email benutzen, ohne Notification Center. Einfach nur zum zu bestätigen, dass es am SMTP liegt.

@baslerbikes
Copy link

Die Idee habe ich schon verstanden, nur wie halte ich das NFC dabei raus.
Bzw. tue ich das nicht durch absenden des Formulars.
Ich verstehe es nicht ...tut mir leid
Ich habe hier noch ein einfaches Formular
http://test.droplimits.de/academy.html
Das hat die ContaoAcademy angelegt.
Nutze ich das, dauert der Aufruf der Weiterleitungsseite 2,7sec

@fritzmg
Copy link
Contributor

fritzmg commented Mar 25, 2019

Die Idee habe ich schon verstanden, nur wie halte ich das NFC dabei raus.

Einfach deinstallieren.

Ich habe hier noch ein einfaches Formular
http://test.droplimits.de/academy.html
Das hat die ContaoAcademy angelegt.
Nutze ich das, dauert der Aufruf der Weiterleitungsseite 2,7sec

Und damit wird auch eine Email über den SMTP versendet?

@baslerbikes
Copy link

Okay NFC gelöscht.

Aufruf der Weiterleitungsseite dauert jetzt mehr als 16sec, teilweise 20sec .. also noch länger :-(
email kommt an
und erstaunlicher Weise werden die Inserttags weitergegeben ... aber da ist ja vielleicht die weiter oben erwähnte Anpassung schon am Werk?

@baslerbikes
Copy link

Beim ContaoAcademy Formular verlängert sich die Zeit auf 24 sec
wenn ich den Formularversand per eMail einstelle

@baslerbikes
Copy link

baslerbikes commented Mar 25, 2019

die parameters.yml sieht so aus

parameters:
database_host: sql635.your-server.de
database_port: 3306
database_user: JB_Contao
database_password: xxx
database_name: JB_Contao
secret: xxx
mailer_host: mail.your-server.de
mailer_user: jb@droplimits.com
mailer_password: xxx
mailer_port: 587
mailer_encryption: tls

fehlt da nicht noch sowas wie
mailer_transport: smtp

@contaoacademy
Copy link

Ergänze den Eintrag um
mailer_transport: smtp

Was passiert dann?

@baslerbikes
Copy link

Ich habe das mal unten drunter geschrieben und keinen Unterschied festgestellt.
Mit "mail" habe ich es auch schon probiert.
Ich vermute, dass ich wieder irgendeine Form/ richtige Platzierung nicht beachtet habe in der das eingetragen werden muss

@fritzmg
Copy link
Contributor

fritzmg commented Mar 26, 2019

Ich habe das mal unten drunter geschrieben und keinen Unterschied festgestellt.

Hast du danach auch den Symfony Application Cache neu aufbauen lassen? Ohne dieser Angabe hattest du eigentlich bisher nie einen SMTP Server benutzt.

@baslerbikes
Copy link

Beim Deinstallieren habe ich den Cache aufgebaut. Hier bin ich mir jetzt nicht sicher. Vielleicht komme ich heute Nacht nochmal dazu es zu probieren. Dazu aber nochmal die Frage:
Muss ich in der Form/ Schreibweise und/ der Position etwas zwingend beachten?

@baslerbikes
Copy link

Kleiner Fehler, große Wirkung!
Jetzt erfolgt die Weiterleitung unter 3 sec und die inserttags werden weitergegeben.

Danke an die Helfer und Tippgeber!

bei mir bleibt aber eine Frage:
Ich hatte das Problem hier schon einmal besprochen und die Änderung durchgeführt
https://community.contao.org/de/showthread.php?74243-NotificationCenter-inserttags&p=498065&viewfull=1#post498065

Wie und warum wurde die Einstellung wieder revidiert? Wo oder wann hätte ich besser aufpassen müssen.
Was mich besonders verwirrt: die SMTP Einstellung war komplett weg, aber der manuell von mir eingetragene Port war noch vorhanden?
Ich habe die "parameters.yml" nach der 1. Änderung nämlich nicht wieder angefasst.
Ich würde es gern verstehen ... damit mir das nicht noch einmal passiert.

@fritzmg
Copy link
Contributor

fritzmg commented Mar 29, 2019

Die parameters.yml wird wenn dann nur vom Contao Install Tool verändert - beim Eintragen der Datenbank Zugangsdaten. Alle anderen Änderungen müssen von dir (oder jemand Anderen) kommen.

Versuche es nochmal mit mailer_transport: sendmail, nur um zu bestätigen, dass der Email Versand über sendmail so lange dauert - und nicht wie von Hetzner vermutet durch Datenbankabfragen.

@baslerbikes
Copy link

Danke dass du mir auch noch den Tipp gegeben hast wie man das Ergebnis verifizieren kann!

Ist positiv verlaufen.
Mit "SMTP" gewünscht Geschwindigkeit, mit "sendmail" wieder elend langsam - allerdings wurden die Insertags trotzdem weitergegeben, was zu Anfang ja auch nicht klappte.
Zurück auf "smtp" ist die Geschwindigkeit wieder in Ordnung!

Vielen, vielen Dank!
Das gleiche Problem hat mir auch bei einer anderen Website zu schaffen gemacht.

@padarom
Copy link

padarom commented Apr 18, 2019

Seems more than a workaround than a real fix for me, but it's better than not replacing the tags 👍

It's not a workaround at all. The session needs to be cleaned after some time. Right now, it remains in in there forever.

I'm pretty sure @benfolds was not referencing clearing the session to be a workaround. He's referencing that a timed clear is not a very precise solution.

When I use a session variable to display a text after a form was submitted then that text will be displayed for every request that's made within the 10 (or however many it's configured to now) seconds. I submit a form, see the message. Go to another page for a second, then come back and I still see "Thanks for submitting the form" or the like.

In other cases the request might legitimately take 10+ seconds because someone runs a lot of logic in a protected area that's not exposed to end users, just to users with certain groups. They are fine with a request taking longer, but the developer might want to display session data that is purged in the meantime because another part of the logic took too long.

Maybe a bit of a contrived example, but my point is that clearing the session basically asynchronously is not an ideal solution for many use cases, and hard to understand and debug too from the developer's perspective who doesn't know that this time limit exists.

@Toflar
Copy link
Member

Toflar commented Apr 23, 2019

That's true. However, there's no way to keep that backwards compatible other than the timed solution. Before, the values were kept in the session forever (for as long as the session was valid). Tehre's no way we can determine when to clear it. A better solution would be some special content element that outputs the form details again and then clears the session. But again, that would not have been backwards compatible.

@padarom
Copy link

padarom commented Apr 23, 2019

That reminds me of my suggestion here.

The issue was noticed by using the {{post::}} insert tag, so I thought it could make sense to add a modifier to that insert tag that just clears the session (or its one specific value) when set. That would remain backwards compatible as long as people haven't defined such a modifier for this insert tag themselves. It's not perfect in that you'd still have to know to use this modifier, but at least it gives you the option to.

I just don't know if this is something that should remain in userland or if it should be included in Contao.

@Toflar
Copy link
Member

Toflar commented Apr 23, 2019

Nobody would use that new flag and the cache would always be disabled. That's not really an option.

@leofeyer leofeyer modified the milestones: 4.6.13, 4.6 May 14, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests