Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Summary follows.
Comment | File | Size | Author |
---|---|---|---|
#31 | mollom.cache_.31.patch | 72.15 KB | sun |
#29 | mollom.cache_.29.patch | 71.63 KB | sun |
#27 | mollom.cache_.27.patch | 67.16 KB | sun |
#26 | mollom.cache_.26.patch | 10.56 KB | sun |
#25 | mollom.cache_.25.patch | 15.17 KB | sun |
Comments
Comment #2
sunQuite a bunch of the test failures were expected, since the enforced caching also has an impact on the fake/testing server implementation.
Now, attached is a pretty radical patch, which attempts to eliminate the entire dependency on the form cache.
Btw, #type 'hidden' is totally weird. I did not know that it does not process user input. (There's no
form_type_hidden_value()
#value_callback function for it.) I don't know why that is, but Drupal core must have thought it is a good idea... (?!)Comment #4
sunResolved a couple of problems caused by #type 'hidden'.
But now with a pretty challenging @todo about tracking state without the luxury of the form cache. Perhaps something along the lines of a token might work?
Comment #6
sunSpent the past days with debugging this to death. Let's see how far we get.
Note: Patch contains tons of commented-out debugging code.
Comment #8
sunExciting!
Now without all of the debugging code.
Comment #10
sunIf this one comes back green, then we're ready for production/live site testing.
That said, this still needs a lot of code documentation, which I'll work on in parallel.
Comment #11
sunAdded tons of documentation and final code clean-ups.
Comment #12
sunTest validation.
Comment #14
sunExcellent. Committed and pushed to 7.x-2.x.
That said, these (major) changes are being manually tested on various production sites currently. Based on my own testing, I continue to expect to receive positive confirmations over the next few days.
Meanwhile, we can move forward with backporting these changes to 6.x-2.x. In case any issues will crop up, we can always get back to D7.
Comment #15
sunAttached patch is a full backport of the D7 patch. It appears to work identically based on my own testing.
Comment #16
sunA couple of additional fixes for D6... but first patch in this serial is still identical to #15.
Comment #17
sunCommitted and pushed to 6.x-2.x.
For now, I'm calling this fixed. If anyone experiences any kind of issues through manual testing prior to the new release, please re-open.
Comment #18
sunWe identified a couple of minor bugs in manual testing — e.g., the CAPTCHA form element is not hidden under certain circumstances.
Comment #20
sunFixed bogus tests.
Comment #21
sunReverted the CAPTCHA URL changes and commented out test assertions, deferred to #1959904: Retrieve only one CAPTCHA per form submission attempt
Comment #22
sunComment #23
sunPostponed #22 and the backport to D6 for now.
Manual testing on production sites discovered some problems that can occur for anonymous users with enabled page + form caching, when repetitively re-validating a form submission. Such form submission flows happen rarely, but yet, they are possible.
Attached patch quadruples the test coverage to reliably catch all of these scenarios.
Comment #25
sunAdjusted the Comment form integration test expectations accordingly. This patch should come back green.
Comment #26
sunAdditionally massively hardening CAPTCHA-only test coverage.
Comment #27
sunCommitted #25 + #26 to 7.x-2.x.
Now back to the backport... (which caused the 7.x-2.x follow-up patches in the first place)
Let's see what the testbot has to say on this cumulative patch serial.
Comment #29
sunIn D6, Comment module triggers the form validation stack from within an #after_build callback, before the form is fully built, and without going through
drupal_process_form()
.Catering for that case required some advanced measures. As far as I can see, this should come back green.
Comment #31
sunHopefully for real now.
Comment #32
sunThanks for reporting, reviewing, and testing! Committed to all branches.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.
Comment #34
aminebourkadi CreditAttribution: aminebourkadi commenteddoes that patch work for D7 version?