Use %language wildcard argument for menu router items

Tagging for the novice queue.

I've changed the hook_menu in locale.module to include the new menu wildcard patterns.
I have also change the
$items['admin/config/regional/translate/delete/%translation']
menu item to directly call the dlete form instead of calling a page with the validation inline. This validation has of course been moved into the translation_load function.

I have added the hook_load functions into the locale.inc file.

I changed the locale_translate_edit_form to accept the full $source object loaded from a single row in the locales_source table so that it matches the param structure from locale_translate_delete_form and they can use a common hook_load.

This is my first patch submit so I'm sure there will be problems and potential re-rolls.

+++ includes/locale.inc.new	2009-12-28 16:50:33.000000000 -0600
@@ -3298,3 +3291,35 @@ function locale_get_localized_date_forma
+  $result = db_query("SELECT language FROM {languages} WHERE language = :language", array(':language' => $langcode))->fetchObject();
+  return $result->language;

Seems backwards to not provide the full language object. I'm sure we probably have to do additional loading on all of the pages that require the this menu loader. Can we simplify this?

+++ includes/locale.inc.new	2009-12-28 16:50:33.000000000 -0600
@@ -3298,3 +3291,35 @@ function locale_get_localized_date_forma
+function translation_load($lid) {
+  if ($source = db_query('SELECT source, context, textgroup, location, lid FROM {locales_source} WHERE lid = :lid', array(':lid' => $lid))->fetchObject()) {
+      return $source;
+  }
+  else {
+      drupal_set_message(t('String not found.'), 'error');
+      drupal_goto('admin/config/regional/translate/translate');
+  }
+}

We don't put this logic in any kind of menu loaders. Either return the object or FALSE.

This review is powered by Dreditor.

I changed the two loader functions to load all fields.
like this:

function language_load($langcode) {
  return $language = db_query("SELECT * FROM {languages} WHERE language = :language", array(':language' => $langcode))->fetchObject();
}

I moved the conditional logic back into the forms.

I had to rework the forms using these loaders a bit. They were previously loading the infomation they needed in the forms and the code had to be change to use a full object instead of a single variable.
like this:

$form['langcode'] = array('#type' => 'value', '#value' => $langcode);

was changed to this

$form['langcode'] = array('#type' => 'value', '#value' => $language->language);

I had also previously submitted an incomplete patch which I also fixed. (I didn't include a variable change in the patch which probably caused an undeclared variable error.)

Sorry. Here's the real patch.

Sorry for the delay. Holidays and all.

I'm having problems with simpletest + MAMP which are impeding the progress here.

I have moved the load functions from locale.inc where they do not belong into locale.module where they actually work.

much better, but still missed a $lid reference.

I believe this patch is now officially ready.
There were two tests still failing because the test was looking for a custom error page. With the %wildcard autoloader all bad urls should goto page not found.
I have modified locale.test to reflect this change.

-    return drupal_not_found();
+    return druapl_not_found();

Oops ;-)

Also, I don't think you can just copy return drupal_not_found() to the locale_translate_delete_form() form constructor function, which is supposed to return a form array, not a string of html. Best replaced with

drupal_not_found();
drupal_exit();

+/**
+ * Auto load language from url wildcard.

+/**
+ * Auto load a translation from url wildcard.

Object loaders are not just for "auto loading" by the menu system.
How about:

Load a [object name] object from the database.

Thank you for the input.
This issue has been completely muddled by #480424: Update locale module to use drupal_static().
I'm going to look at the way they're using drupal_static and try to work this out.
I have high hopes that a solution will be here in a couple days. At the very least the wildcard loaders will be able to be implemented for the menu callbacks, if not for all the other places languages and string translations are loaded.

Cleaning-up the "locale module" issue queue as per http://cyrve.com/criticals.

Um...this applies to the locale.module, not the language system. :)

@Dave Reid

I just talked with Gabor: we agreed to move to locale module the string-translation issues, while moving to the language system queue the language management (add/remove) and negotiation issues.

There is not always a clean distinction, in this case in this case probably I misunderstood the issue ;)

Btw:

+++ includes/locale.inc.modified	2010-01-04 13:40:18.000000000 -0600
@@ -1532,26 +1532,22 @@ function locale_translate_edit_form_subm
+    return druapl_not_found();

typo

Powered by Dreditor.

+++ modules/locale/locale.module.modified	2009-12-30 16:22:33.000000000 -0600

Patch seems to be bogus/reversed.

+++ modules/locale/locale.module.modified	2009-12-30 16:22:33.000000000 -0600
@@ -1123,3 +1126,29 @@ function locale_form_comment_form_alter(
+function language_load($langcode) {

This loader belongs into language.inc.

+++ modules/locale/locale.module.modified	2009-12-30 16:22:33.000000000 -0600
@@ -1123,3 +1126,29 @@ function locale_form_comment_form_alter(
+function translation_load($lid) {

This function needs to be within the Locale module namespace.

Powered by Dreditor.

#15: locale_menu_wildcard.patch queued for re-testing.

Hi @Marc DeLay,

Thanks for taking this on. Are you still working on this issue? If not, we'll unassign it in a day or two so that someone else can give it a try. (Feel free to assign it back to yourself if you'd still like to work on it, as well.) Thanks!

Putting it back in the queue.

language module was already done, modified menu paths only in locale.module hook_menu.

Reviewed the patch the menu items this affects doesn't exist.
So there is actually no need for any patch to this issue.

For details see #1301040: Move language listing functionality from locale.module to a new language.module

They still exist, but are (mostly) located in Language module now.

It seems to me this issue is no longer relevant, since d8 has now switched to using the routing system ?

bad tag formatting, sorry

Fixed for D7, obsolete for D8.