What are the steps required to reproduce the bug?
n/a

What behavior were you expecting?
The Message should be translated in multi-site environment.

What happened instead?
The message was not translated.

Proposed resolution
In color.install file, Line 58, the $message argument to drupal_set_message() should be enclosed within t() so that it is translatable.

Also, fixed minor coding standard issue in modules/color/color.test

Patch is provided in next comment.

CommentFileSizeAuthor
#11 color-Fixed_minor_coding_standard-2779379-10-7.x.patch844 bytessandip27
#2 color-Fixed_t_function_missing_in_.install_file-2779379-2-7.x.patch1.85 KBsandip27

Comments

sandip27 created an issue. See original summary.

sandip27’s picture

sandip27 commented
Status: Active » Needs review
StatusFileSize
new color-Fixed_t_function_missing_in_.install_file-2779379-2-7.x.patch1.85 KB

Please find attached, patch for the same.

sandip27’s picture

sandip27 commented
Issue summary: View changes
sandip27’s picture

sandip27 commented
Assigned: sandip27 » Unassigned
sandip27’s picture

sandip27 commented
Priority: Normal » Critical
panshulk’s picture

panshulk commented
Assigned: Unassigned » panshulk
Status: Needs review » Reviewed & tested by the community

Hi @sandip27

Thanks a lot for reporting and picking this up, I have applied the patch above on 7.x-dev . The patch works as expected .

Below are the fixed results of the patch:

drupal_set_message(t("Some of the custom CSS color codes specified via the color module are invalid. Please examine the themes which are making use of the color module at the @url page to verify their CSS color values.", array('@url' => l(t('Appearance settings'), "admin/appearance/settings"))), 'warning');

if ($is_valid) {
$this->assertText('The configuration options have been saved.');
}

Hence moving this to RTBC

Thank You :)

panshulk’s picture

panshulk commented
Assigned: panshulk » Unassigned
stefan.r’s picture

stefan.r commented
Priority: Critical » Normal
David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Needs work

https://www.drupal.org/docs/7/api/localization-api/when-to-use-st-t-and-... says not to use t() in update functions. Based on that the current code is correct. (Although I suppose there is some debate about that - #1168966: Document not using t() in update functions.)

Also:

-        drupal_set_message('Some of the custom CSS color codes specified via the color module are invalid. Please examine the themes which are making use of the color module at the <a href="'. url('admin/appearance/settings') .'">Appearance settings</a> page to verify their CSS color values.', 'warning');
+        drupal_set_message(t("Some of the custom CSS color codes specified via the color module are invalid. Please examine the themes which are making use of the color module at the @url page to verify their CSS color values.", array('@url' => l(t('Appearance settings'), "admin/appearance/settings"))), 'warning');

Normally we would not move "Appearance settings" out into its own string, but rather leave it in the main string (as the original code did) so it can all be translated at once. See https://www.drupal.org/docs/7/api/localization-api/dynamic-or-static-lin....

-      if($is_valid) {
+      if ($is_valid) {

This change looks good. We could at least do that on its own, even though it's not related to the other issue :)

sandip27’s picture

sandip27 commented
Title: $message argument to drupal_set_message() should be enclosed within t() » Minor Coding Standard Issue in .install file
Issue summary: View changes
Status: Needs work » Needs review

@David_Rothstein, Fair enough.

Please find attached revised patch for 

-      if($is_valid) {
+      if ($is_valid) {
sandip27’s picture

sandip27 commented
StatusFileSize
new color-Fixed_minor_coding_standard-2779379-10-7.x.patch844 bytes
David_Rothstein’s picture

David_Rothstein commented
Title: Minor Coding Standard Issue in .install file » Minor Coding Standard Issue in color.test file
Status: Needs review » Reviewed & tested by the community

Looks good - thanks!

I checked Drupal 8 and this coding standards issue is already fixed there.

stefan.r’s picture

stefan.r commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x, thanks!

  • 223cd34 committed on 7.x 
    Issue #2779379 by sandip27: Minor Coding Standard Issue in color.test...

Status: Fixed » Closed (fixed)

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