Field label is required when creating field so no need to check existence and use $field_name or $delta in block subject and admin title of the block

Also some code clean-up, query for block title should use placeholder %s

CommentFileSizeAuthor
cck_blocks-translate.patch2.63 KBandypost

Comments

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

Thank you for antother useful patch. I agree, that field labels are mandatory, so we don't need to check them for existence.

Could you explain, why you're still checking that?

        if (isset($fields[$delta]['widget']['label'])) {
          $block['subject'] = t($fields[$delta]['widget']['label']);
        }

Just seems to be a different notation of my code without replacing them by the field's name, if the label doesn't exist.

I removed your if-statement and it seems to work.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Hm, I think it's useless

I leave this for case if some contrib module could alter widget settings and make label empty so using isset() is wrong - should be !empty()

andypost’s picture

andypost
he/him
Primary language Russian
commented

Also this patch closes a security hole! Direct user input printed without filtering!

Anonymous’s picture

Anonymous (not verified) commented

Thank's for that comment. Is it possible, that other modules set the label to "NULL"? We have to assign a subject for the block. If that isn't possible, I'd recommend only to do $block['subject'] = t($fields[$delta]['widget']['label']); or leave it like I did it before, as it assigns the name of the field if there is no label.

Your security fix will be commited anyway!

andypost’s picture

andypost
he/him
Primary language Russian
commented

I think it's a bad idea to output $fieldname as block title so let's proceed with $block['subject'] = t($fields[$delta]['widget']['label']);

Anonymous’s picture

Anonymous (not verified) commented
Status: Reviewed & tested by the community » Fixed

Fixed in next -dev

Status: Fixed » Closed (fixed)

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