Problem/Motivation
The initial ajax response (when clicking 'Next' the first time) doesn't seem to contain an AnnounceCommand. It also generates a PHP session for the anonymous user. This is troublesome because users with open sessions (SESS* cookie) will normally bypass CDNs and other full page caching systems (can happen if the user navigates away from a form when they have a session).
I'm thinking this might be a two-birds-with-one-stone issue.
- Ajax wizard forms do not seem to consistently utilize Drupal.announce as expected.
- Every other step through a wizard form as an anonymous user generates a PHP session for said user.
Steps to reproduce
What I would expect to happen is:
- Load up an ajax wizard form.
- Assert that
#drupal-live-announceis empty - Click 'Next'
- Assert that
#drupal-live-announcenow contains"Test: Webform: Wizard basic: Step 2" loaded. (2 of 4) - Click 'Previous
- Assert that
#drupal-live-announcenow contains"Test: Webform: Wizard basic: Step 1" loaded. (1 of 4)
What really seems to happen is:
- Load up an ajax wizard form.
#drupal-live-announceis empty- Click 'Next'
#drupal-live-announceis empty- Click 'Previous
#drupal-live-announcenow contains
"Test: Webform: Wizard basic: Step 2" loaded. (2 of 4) "Test: Webform: Wizard basic: Step 1" loaded. (1 of 4)
Proposed resolution
I think the path forward here might involve not storing the announcement in the user's session, but rather in memory (perhaps drupal_static or \Drupal\Core\Cache\MemoryCache\MemoryCache?
Remaining tasks
Make the patch beautiful. The service should probably be injected somehow / somewhere. I wasn't sure what the best way to approach it was because the service is needed within a trait. Procedurally grabbing the service is not an optimal approach.
Does the test require a new test form, or is there an existing one that can/should be used? If an existing form can be used, then there'd be less patch noise.
User interface changes
None
API changes
Perhaps a new service if the memory cache service is preferable, otherwise none if drupal_static is used.
Data model changes
None
| Comment | File | Size | Author |
|---|---|---|---|
| #24 | 3178298-24.patch | 3.19 KB | jrockowitz |
| #23 | 3178298-23.patch | 1.75 KB | jrockowitz |
| #19 | 3178298-ajax-announce-19.patch | 9.36 KB | luke.leber |
Comments
Comment #2
luke.leberComment #3
luke.leberComment #4
luke.leberComment #5
luke.leberComment #6
luke.leberIt really does help if the test group is spelled correctly...time for a nap.
Comment #7
luke.leberComment #8
luke.leberComment #9
luke.leberUpdate - I think that I've been able to track down a contributing factor to this conundrum.
\Drupal\webform\src\Form\WebformAjaxFormTrait::setAnnouncementsseems to initiate a session for would-be anonymous users (sometimes).As a result, when a user first visits a form, they do not have a PHP session. If they proceed to the second step, then a session is created for them and the announcement seems to be stored in it. If they proceed again, then the announcement seems to be picked up and sent back as part of the ajax request. The session is then terminated (nothing left in it after
\Drupal\webform\src\Form\WebformAjaxFormTrait::resetAnnouncementsis called!I think the path forward here might involve not storing the announcement in the user's session, but rather in memory (perhaps
drupal_staticor\Drupal\Core\Cache\MemoryCache\MemoryCache?Switching from session based storage to the memory cache seems to take care of the original problem.
I'll try to get a patch uploaded this weekend.
I think the title is a bit confusing -- I might only be seeing a symptom of an underlying problem with Drupal.announce not working properly.
Comment #10
luke.leberComment #11
luke.leberI don't think that the new test is going to run...but hopefully I've added enough info to this issue to help!
Cheers!
Comment #12
luke.leberComment #13
luke.leberComment #14
luke.leberComment #15
luke.leberComment #16
luke.leberComment #17
luke.leberComment #18
luke.leberRemoving Needs a11y review tag -- I actually think this is for core issues only...but an accessibility review wouldn't hurt :)
Comment #19
luke.leberFixed namespace in the new test...
Comment #20
luke.leberComment #21
luke.leberComment #22
luke.leberComment #23
jrockowitz commentedMemoryCache is not unique to each user and concurrent users might receive the wrong announcement.
Using a static property is not working as expected. (See attached patch)
Comment #24
jrockowitz commentedUsing drupal_static works because the announcement property is being serialized with the form object via the Ajax request.
I don't think we need a new dedicated webform to test this behavior and I moved the new tests into WebformWizardBasicJavaScriptTest
Comment #25
luke.leberThat patch looks awesome and seems to resolve the originally reported issue. +1 RTBC from me.
Comment #26
luke.leberComment #27
jrockowitz commented