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.
Steps to reproduce:
1. Create content for a node type: say Article with an image (though the bug is for any content type and any field filled with data)
2. Now navigate to admin/structure/types/manage/article/fields/field_image/field-settings (with or without the overlay) to change field settings
Drupal informs with the error "There is data for this field in the database. The field settings can no longer be changed" .... but message is hidden because of the message-24-error.png image repeated.
See attached image.
Comment | File | Size | Author |
---|---|---|---|
#13 | drupal-938672-13.patch | 1.01 KB | tim.plunkett |
#4 | field-ui-error-messages_938672.patch | 975 bytes | Jeff Burnz |
#4 | field-ui-error.png | 9.83 KB | Jeff Burnz |
screenshot_002.png | 69.46 KB | FanisTsiros |
Comments
Comment #1
Jeff Burnz CreditAttribution: Jeff Burnz commentedThere is no accounting for errors in field-descriptions, the icon gets applied but that's all - no border, background, padding, background-repeat etc.
Needs work right away.
Comment #2
yoroy CreditAttribution: yoroy commented"Interesting"
Comment #3
webchickK, well this is just dumb. good catch.
Comment #4
Jeff Burnz CreditAttribution: Jeff Burnz commentedIssues is in field_ui.module, the "messages" class is omitted (which carries the important generic styles inherited by all messages).
With patch:
Comment #5
FanisTsiros CreditAttribution: FanisTsiros commentedOK RTBC from me.
Works fine now ! I'll let status unchanged if someone else want to review...
thanks !
Comment #6
tim.plunkettWasn't there an issue similar to this before? Hundreds of red Xs!
Good catch, good patch!
Comment #7
webchickWell. This works, so committed to HEAD.
But I'm wondering if we need to re-think how that red X is styled. This is at least the second place it's come up in core alone. It seems like contributed modules are going to hit this everywhere. Shouldn't the default styling be sane?
Comment #8
Jeff Burnz CreditAttribution: Jeff Burnz commentedThe default styling is sane - the alternative is to apply, repetitively, the same CSS to every message type explicitly. Asking us to do this strips away one of the most powerful tools we have - the cascade.
drupal_set_message adds the .message class automatically, but this issue was because the message is hard coded instead of calling drupal_set_message. If for some reason it can't use the function then surely the burden falls on the developer to be sure they emulate the output of the function, else there's always going to be trouble. The mistake here is not with the CSS, its with the use of the CSS.
If contrib module developers ignore the API that's at their peril - use drupal_set_message or understand what needs to be done if you don't/can't.
Comment #9
webchickOk, thanks. That makes sense. So as long as contrib is calling the standard drupal_set_message() function, they won't run into this, and field UI is just being weird. :P
Comment #10
tobiasbI#m not sure, but the "div"
t('There is data for this field in the database. The field settings can no longer be changed.' . '</div>')
shouldn#t be inside the t()-function. Because this isn#t something, you would want to translate.Comment #11
FanisTsiros CreditAttribution: FanisTsiros commented@tobiasb
why to prevent translation for this message ?
I am a core translator and there was a debate in our translation team if and how we should translate error messages in general. The decision was to translate everything, regardless these messages are for the end users, or for the sites administrators.
The main issue was "Ok i see an error message, what is the english message to ask, or search for help in d.o. ?"
but if we are not going to translate messages like these, does it make sense to translate administrator interface at all?
I am the one to support "translate everthing " ! ( i also wish we had i18n in core, but this wont happen for D7...:-(
I'm not going to change status if someone want to discuss further...
Comment #12
Jeff Burnz CreditAttribution: Jeff Burnz commented@FanisTsiros - I think you mistake the report in #10, what he means is that the closing bracket for t() is misplaced (indeed it looks like a typo):
Surely this is meant to be:
Comment #13
tim.plunkettThe case of the misplaced parenthesis has been solved.
Comment #14
tobiasb@Jeff Burnz right, thx.
Comment #15
FanisTsiros CreditAttribution: FanisTsiros commentedHey, it seems i'm blind, sorry Tobias.
Comment #16
webchickCommitted to HEAD. Thanks!