String freeze: Move HTML out of strings wherever possible.

Excellent! :D Thanks so much, Gurpartap!

Also updating this to critical, as it's a string-freeze thing.

one coding-style issue I notice off the bat:

) . '</p>;

should be:

) . '</p>;

(no space before the '

')

There are some other inconsistencies which your patch has highlighted, I'll go through and make another attempt once you post your new patch to include install.php.

Wow, great!!!

There are still a couple minor nits, but overall I think this sucker is ready to go:

install.php

-  print theme('install_page', st('<ul><li>To start over, you must empty your existing database.</li><li>To install to a different database, edit the appropriate <em>settings.php</em> file in the <em>sites</em> folder.</li><li>To upgrade an existing installation, proceed to the <a href="@base-url/update.php">update script</a>.</li></ul>', array('@base-url' => $base_url)));
+  print theme('install_page', '<ul>'. st('<li>To start over, you must empty your existing database.</li><li>To install to a different database, edit the appropriate <em>settings.php</em> file in the <em>sites</em> folder.</li><li>To upgrade an existing installation, proceed to the <a href="@base-url/update.php">update script</a>.</li>', array('@base-url' => $base_url))) .'</ul>';

I'd keep the <ul> in there, as it adds to the context (this is an unordered list, rather than an ordered one).

-    $output .= '<p>' . (drupal_set_message() ? st('Please review the messages above before continuing on to <a href="@url">your new site</a>.', array('@url' => url(''))) : st('You may now visit <a href="@url">your new site</a>.', array('@url' => url('')))) . '</p>';
+    $output .= '<p>' . (drupal_set_message() ? st('Please review the messages above before continuing on to <a href="@url">your new site</a>.', array('@url' => url(''))) : st('You may now visit <a href="@url">your new site</a>.', array('@url' => url('')))) .'</p>';

Remove space after <p>

aggregator.module

Ah, ok. This was my fault. I wasn't specific enough about removing \"s..

Stuff like:

-  $output = "<div id=\"aggregator\">\n";
+  $output = '<div id="aggregator">'. "\n";

and

-  $output .= "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n";
-  $output .= "<rss version=\"2.0\">\n";
+  $output .= '<?xml version="1.0" encoding="utf-8"?>'. "\n";
+  $output .= '<rss version="2.0">'. "\n";

The originals here are actually fine; we're only caring about \" stuff in t() functions; no translator will ever look at these strings, so they can stay escaped so that we don't need concatenation which is expensive.

comment.module

-      return t("<p>Below is a list of the latest comments posted to your site. Click on a subject to see the comment, the author's name to edit the author's user information , \"edit\" to modify the text, and \"delete\" to remove their submission.</p>");
+      return '<p>'. t('Below is a list of the latest comments posted to your site. Click on a subject to see the comment, the author\'s name to edit the author\'s user information , "edit" to modify the text, and "delete" to remove their submission.') .'</p>';

Since the double-quotes here aren't required (as they are in HTML), here's a place I would just re-word the string to say 'edit' instead of "edit"

drupal.module

-      return variable_get('drupal_authentication_service', 0) ? t("<p><a href=\"@Drupal\">Drupal</a> is the name of the software that powers %this-site. There are Drupal web sites all over the world, and many of them share their registration databases so that users may freely log in to any Drupal site using a single <strong>Drupal ID</strong>.</p>
-<p>So please feel free to log in to your account here at %this-site with a username from another Drupal site. The format of a Drupal ID is similar to an e-mail address: <strong>username</strong>@<em>server</em>. An example of a valid Drupal ID is <strong>mwlily</strong>@<em>drupal.org</em>.</p>", array('@Drupal' => 'http://drupal.org', '%this-site' => variable_get('site_name', 'this web site'))) : '';
+      return variable_get('drupal_authentication_service', 0) ? '<p>'. t('<a href="@Drupal">Drupal</a> is the name of the software that powers %this-site. There are Drupal web sites all over the world, and many of them share their registration databases so that users may freely log in to any Drupal site using a single <strong>Drupal ID</strong>.</p>
+<p>So please feel free to log in to your account here at %this-site with a username from another Drupal site. The format of a Drupal ID is similar to an e-mail address: <strong>username</strong>@<em>server</em>. An example of a valid Drupal ID is <strong>mwlily</strong>@<em>drupal.org</em>.' .'</p>', array('@Drupal' => 'http://drupal.org', '%this-site' => variable_get('site_name', 'this web site'))) : '';

This one is borderline, but since the </p> is in the string, I would include the <p> in it too.

I have to run now; can continue looking at this later tonight. I've incorporated these comments, though, so up through drupal.module should be fixed.

marking needs work, as the rest of this needs looking at. It's very close, though!

Great!!

filter.module

-      return t('
-<p>If you cannot find the settings for a certain filter, make sure you\'ve enabled it on the <a href="@url">view tab</a> first.</p>', array('@url' => url('admin/settings/filters/'. arg(3))));
+      return '<p>'. t('
+If you cannot find the settings for a certain filter, make sure you\'ve enabled it on the <a href="@url">view tab</a> first.' .'</p>', array('@url' => url('admin/settings/filters/'. arg(3))));
 

For this, I just turned "you've" into "you have" to eliminate the \'

-                'abbr' => array( t('Abbreviation'), t('<abbr title="Abbreviation">Abbrev.</abbr>')),
-                'acronym' => array( t('Acronym'), t('<acronym title="Three-Letter Acronym">TLA</acronym>')),
+                'abbr' => array( t('Abbreviation'), '<abbr title="Abbreviation">'. t('Abbrev.') .'</abbr>'),
+                'acronym' => array( t('Acronym'), '<acronym title="Three-Letter Acronym">'. t('TLA') .'</acronym>'),

I actually reverted this change; if people change "Abbrev." they are likely going to want to change the long version too.

search.module

-      return t('<ul>
-<li>Check if your spelling is correct.</li>
+      return '<ul>'. t('<li>Check if your spelling is correct.</li>
 <li>Remove quotes around phrases to match each word individually: <em>"blue smurf"</em> will match less than <em>blue smurf</em>.</li>
-<li>Consider loosening your query with <em>OR</em>: <em>blue smurf</em> will match less than <em>blue OR smurf</em>.</li>
-</ul>');
+<li>Consider loosening your query with <em>OR</em>: <em>blue smurf</em> will match less than <em>blue OR smurf</em>.</li>') .'</ul>';

Reverted this one too... Same thing as before; let's keep the <ul> in the string for context.

system.module

-      return t('<p>Welcome to the administration section. Here you may control how your site functions.</p>');
+      return '<p>'. t('p>Welcome to the administration section. Here you may control how your site functions.') .'</p>';

oopsie. ;) Fixed.

-    $description .= '<p>'. t("<strong class=\"ok\">Currently, all enabled modules are compatible with the aggressive caching policy.</strong> Please note, if you use aggressive caching and enable new modules, you'll need to check this page again to ensure compatibility.") .'</p>';
+    $description .= '<p>'. t('<strong class="ok">Currently, all enabled modules are compatible with the aggressive caching policy.</strong> Please note, if you use aggressive caching and enable new modules, you\'ll need to check this page again to ensure compatibility.') .'</p>';

same as before, broke "you'll" into "you will"

Other than those very, very minor things, I couldn't find anything else wrong with this patch. Rolled again, with these changes. I'd love for one more person to look at this, and then we can mark RTBC.

marked http://drupal.org/node/97897 and http://drupal.org/node/97894 as duplicates of this.

part of http://drupal.org/node/97889 is duplicative, but not all.

This one has a few changes that ChrisKennedy found: a double-space in filter.module (not by this patch, but might as well fix it), and a couple of instances where the closing </p> was put inside the t().

Ah, uploaded too soon. :) There was also a grammatical error in user help:

"administrators to register a new users by hand"

changed to:

"administrators to register new users by hand"

Thanks, ChrisKennedy!

Btw, one other thing Chris found was that the default values of certain variable_gets were untranslated. Threw t()s around:

theme.inc, comment.module, node.module, system.module, user.module

$output = variable_get('anonymous', 'Anonymous');

put t() around Anonymous.

user.module

$site = variable_get('site_name', 'this website');

enclosed in t()

aggregator.moule

variable_get('site_name', t('Drupal'))

Actually I removed the t() here, since all other instances of it werent' t()d and "Drupal" is "Drupal" in any language. ;)

drupal.module

'%this-site' => variable_get('site_name', 'this web site')

t()ed and changed "web site" to "website" since that's the word we use everywhere (I know most style guides recommend "web site" or "Web site" but most *users* search for "website" and there are twice as many instances of "website" in core as "web site," so there you go.) I also went through core and changed any instances of "web site" to "website" (there were about 10 of these, vs. 30-something of "website")

One final push for the evening; this includes some minor fixes that neclimdul found... some strings that began with newlines, a couple places where "'something' blah" was done that could've just as easily been '"something" blah' so was switched, etc.

Btw, could the next person who reviews this please start from the BOTTOM and work their way up? I have a feeling there might be things we're missing down there because we're just plain too tired by the time we get to like watchdog module. ;)

in the function contact_admin_edit() there are a couple strings that could have their quotes swapped.

Line 171:

    '#description' => t("Example: 'website feedback' or 'product information'."),

Line 177:

    '#description' => t("Example: 'webmaster@yoursite.com' or 'sales@yoursite.com'. To specify multiple recipients, separate each e-mail address with a comma."),

I'll look this over later, when I get back from school. There were a few minor things I noticed when I gave it a very, very brief look over right now, but I'll go into more details when I get back.

I'd like to suggest doing one patch at a time by type of change. Scanning through and looking at moving tags is easy, as is changing quotes. But looking at both at once with some more case changes is a bit overwhelming.

Ok, coming up.

Ok. I think all of these have been moved out into other patches. Renaming this one to be more accurate of what this started out being a patch about.

greggles has stepped up to take a look at the patch in #4 and strip out anything that doesn't fall under this purview. I'll take another quick look at the latest one and see if there's anything else that needs a patch rolled for it.

Ok, here is an attempt at doing just the

and related kinds of changes. I tried to keep out any of the \' or \" changes and I used my best judgment on when to leave the

in the translation.

I found an error (somehow a 'p> got left in a string) and I almost certainly introduced some new errors.

Reviews requested.

Also, the patch in #20 was based on Gurpartap's patch from #4 - that was Webchick's advice.

In looking through, I noticed a couple things still in this patch that were cleaned up in subsequent patches. Gimmie a few mins...

I *think* that's all of them. Marking back for review.

That stray 'p> is back in book.module again. haven't looked at the rest yet.

Checked over once more with a fine-toothed comb; looks good. marking RTBC.

Re-roll. Book.module's hunk failed, but that's because the text there was rewritten so it's no longer needed.

No longer applies and the first two chunks have spacing issues. (Always put a space between . and any non-quote.)

Committed to HEAD, although all chunks did not apply.

Thanks, Neil!

Here's the remainder of those hunks that failed.

Committed to HEAD.