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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jeff Burnz’s picture

Title: Field settings page broken with "field settings can no longer be changed" error » fieldset-description error messages need work
Version: 7.0-beta1 » 7.x-dev
Component: Seven theme » system.module
Priority: Normal » Major

There 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.

yoroy’s picture

"Interesting"

webchick’s picture

Issue tags: +beta blocker

K, well this is just dumb. good catch.

Jeff Burnz’s picture

Component: system.module » field_ui.module
Status: Active » Needs review
Issue tags: +Quick fix
FileSize
9.83 KB
975 bytes

Issues is in field_ui.module, the "messages" class is omitted (which carries the important generic styles inherited by all messages).

With patch:

field-ui-error.png

FanisTsiros’s picture

OK RTBC from me.
Works fine now ! I'll let status unchanged if someone else want to review...
thanks !

tim.plunkett’s picture

Status: Needs review » Reviewed & tested by the community

Wasn't there an issue similar to this before? Hundreds of red Xs!

Good catch, good patch!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Well. 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?

Jeff Burnz’s picture

The 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.

webchick’s picture

Ok, 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

tobiasb’s picture

Status: Fixed » Needs work

I#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.

FanisTsiros’s picture

@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...

Jeff Burnz’s picture

@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):

   $form['field']['#description'] = '<div class="messages error">' . t('There is data for this field in the database. The field settings can no longer be changed.' . '</div>') . $form['field']['#description'];

Surely this is meant to be:

   $form['field']['#description'] = '<div class="messages error">' . t('There is data for this field in the database. The field settings can no longer be changed.') . '</div>' . $form['field']['#description'];
tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
1.01 KB

The case of the misplaced parenthesis has been solved.

tobiasb’s picture

Status: Needs review » Reviewed & tested by the community

@Jeff Burnz right, thx.

FanisTsiros’s picture

Hey, it seems i'm blind, sorry Tobias.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix, -beta blocker

Automatically closed -- issue fixed for 2 weeks with no activity.