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.- go to admin/structure/types/manage/article/display, make sure selected formatter for 'body' field is 'Default' - if not, set it to 'Default' and hit 'Submit'
- change formatter for 'body' field to 'Trimmed', don't 'Submit' but hit the 'Edit settings' cogwheel button at the end of the 'Body' row.
- instead of the settings form for the 'Trimmed' formatter, you get the one for the 'Default' formatter (which has no settings, thus empty form)
Attached patch updates Field UI code for the new behavior of #limit_validation_errors (rightly) introduced in #763376: Not validated form values appear in $form_state.
I set up to write a test for that, but it turns out that the bug is JS-only, so no amount of simpletesting would have helped catch this.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 896162-field_ui-formatter-settings-11.patch | 851 bytes | effulgentsia |
| #3 | field_ui_limit_validation_errors-896162.patch | 1.61 KB | yched |
| #2 | field_ui_limit_validation_errors-896162.patch | 1.61 KB | yched |
| field_ui_limit_validation_errors.patch | 1021 bytes | yched | |
Comments
Comment #1
yched CreditAttribution: yched commented+ minor : found #896170: default formatter for 'Text with summary' field type should be 'text_default' while debugging this.
Comment #2
yched CreditAttribution: yched commentedUpdated patch. I initially thought it was OK, but the 'Cancel' button within the 'formatter settings' form needs the same fix.
Comment #3
yched CreditAttribution: yched commentedDoh @ copy/paste error in the comment.
Comment #4
tstoecklerShould this not be "Do not check errors for the 'Cancel' button."?
Powered by Dreditor.
Comment #5
yched CreditAttribution: yched commentedre #4 - yup, fixed in #3 (crosspost)
Comment #6
effulgentsia CreditAttribution: effulgentsia commentedI did not test the OD manually, but I trust that yched did. It makes total sense, and there is no down-side in adding a validation requirement, especially if the value is then used by the submit handler, so this is pretty much a no-brainer.
Does this actually require testing of any JS code (i.e., browser-specific quirkiness)? Or simply of a multistep AJAX process? If the latter, please help get this in: #789186: Improve drupalPostAJAX() to be used more easily, leading to better AJAX test coverage. Then, we can add test coverage to this issue.
Comment #7
webchickToo bad about the test coverage. :\ Committed to HEAD. Thanks!
Comment #8
yched CreditAttribution: yched commentedThks webchick.
re eff #6 : a test for this would involve two successive ajax submits, which, IIRC, is out out reach of the current drupalPostAJAX().
An enhanced version that doesn't break the flow by leaving you with a raw array of ajax commands would certainly make this more doable.
Comment #9
effulgentsia CreditAttribution: effulgentsia commentedBeing able to do multiple successive AJAX submits in simpletest is EXACTLY what #789186: Improve drupalPostAJAX() to be used more easily, leading to better AJAX test coverage is about :)
Comment #11
effulgentsia CreditAttribution: effulgentsia commentedWe need more tests for this form, and now that #789186: Improve drupalPostAJAX() to be used more easily, leading to better AJAX test coverage landed, we can write them (whenever someone has the time to do so). In the meantime, another bug that surfaced is when you "Manage Display" of an Article, then change the Image format to "hidden", then back to "Image", you get an AJAX error dialog. Here's a fix for that.
Comment #12
Bojhan CreditAttribution: Bojhan commentedSo can anyone review this and mark it RTBC?
Comment #13
yched CreditAttribution: yched commentedAway from my coding env, cannot write tests right now, but patch #11 is good, syntax for the #limit_validation_errors value is incorrect.
Bug should only have minor consequences in itself.- er, not. Bug causes a PHP fatal error...Comment #14
Alan D. CreditAttribution: Alan D. commentedThe thread seems a bit confusing to me (status wise), but I discovered this bug in a Drupal 7.0-alpha7 (2010-Sep-16 ) install with some custom code. This was post #7 commit (August 29, 2010 - 06:12).
The AJAX error was:
And patch #11 resolved this nicely.
Comment #15
webchickTo stop the critical regression, I have committed this patch to HEAD. But I am not committing any more patches to this area of the code until we get tests here.
Begrudgingly downgrading this to only "major", even though not one but two critical issues surfacing here as a result of holes in test coverage borders on yet another critical bug all by itself.
Comment #16
yched CreditAttribution: yched commented@webchick : I know it's frustrating, sorry about that. Now that the hard refactoring work on Field UI "manage display" is done (and that #789186: Improve drupalPostAJAX() to be used more easily, leading to better AJAX test coverage is in), writing a full fledged test suite for that screen is the next item on my todo, when I come back to business - in a fortnight or so...
Note that #886152: All fields are hidden after the administrator newly configures a view mode to not use 'default' display has an initial set of tests for the screen.
Comment #17
sun.core CreditAttribution: sun.core commentedSince only tests remain, demoting to normal.
Comment #18
effulgentsia CreditAttribution: effulgentsia commentedI agree with the "normal" designation, since I think via other issues, test coverage of this page has already improved anyway. Will let yched decide, however, whether to submit more, or to close the issue.
Comment #19
yched CreditAttribution: yched commentedMore tests are coming up - almost ready, stumbled on a technical issue with submitting forms with image button in tests. Will post more about this when other hot issues have settled down.
Meanwhile, agreed with 'not major'.
Comment #20
Tor Arne Thune CreditAttribution: Tor Arne Thune commented