Contrib.social

Add a test for a hook implementation that defines a langcode but language module is not enabled.

Open on Drupal.org →
Created on 27 March 2013, over 12 years ago
Updated 17 January 2025, 6 months ago

Follow up for 4. in #1658846-187: Add language support to node access grants and records →

Problem/Motivation

If some hook implementation defines a langcode, but language is off or the language is not enabled, it looks like this would blow up.

Proposed resolution

If some hook implementation defines a langcode, but language is off or the language is not enabled, it looks like this would blow up. Did I say that already and ask for a test for it? Edit: No, I think I said maybe it was on the contrib module to add language to its requirements. However, let's make sure we document that somehow, somewhere. If you execute a db_insert() with no values, does it simply insert no records, or does it fail? Can someone test this?

Remaining tasks

  • Find out what happens when execute a db_insert() with no values. Does it simply insert no records, or does it fail?
  • Add comments to the code to document what happens.
  • Add the test for if a module has some hook implementation defining a langcode when language is not enabled. See contributor task document for writing tests: http://drupal.org/node/1468170

User interface changes

No UI changes.

API changes

No API changes.

Original report by @xjm

In 4. in #1658846-187: Add language support to node access grants and records → 

+++ b/core/modules/node/node.moduleundefined
@@ -3036,18 +3077,35 @@ function _node_access_write_grants(Node $node, $grants, $realm = NULL, $delete =
+      if (isset($grant['langcode'])) {
+        $grant_languages = array($grant['langcode'] => language_load($grant['langcode']));
+      }
+      else {
+        $grant_languages = $node->getTranslationLanguages(TRUE);
+      }
+      foreach ($grant_languages as $grant_langcode => $grant_language) {
+        // Only write grants; denies are implicit.
+        if ($grant['grant_view'] || $grant['grant_update'] || $grant['grant_delete']) {
+          $grant['nid'] = $node->nid;
+          $grant['langcode'] = $grant_langcode;
+          // The record with the original langcode is used as the fallback.
+          if ($grant['langcode'] == $node->langcode) {
+            $grant['fallback'] = 1;
+          }
+          else {
+            $grant['fallback'] = 0;
+          }
+          $query->values($grant);
+        }

If some hook implementation defines a langcode, but language is off or the language is not enabled, it looks like this would blow up. Did I say that already and ask for a test for it? Edit: No, I think I said maybe it was on the contrib module to add language to its requirements. However, let's make sure we document that somehow, somewhere. If you execute a db_insert() with no values, does it simply insert no records, or does it fail? Can someone test this?

📌 Task
Status

Postponed: needs info

Version

11.0 🔥

Component

node system

Created by

🇺🇸United States yesct

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • D8MI

    (Drupal 8 Multilingual Initiative) is the tag used by the multilingual initiative to mark core issues (and some contributed module issues). For versions other than Drupal 8, use the i18n (Internationalization) tag on issues which involve or affect multilingual / multinational support. That is preferred over Translation.

  • stale-issue-cleanup

    To track issues in the developing policy for closing stale issues, [Policy, no patch] closing older issues

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 6 months ago →
    🇺🇸United States smustgrave

    Thank you for creating this issue to improve Drupal.

    We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

    Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

    Thanks!

  • Status changed to Closed: outdated 4 months ago
  • Comment 4 months ago →
    🇦🇺Australia acbramley

    I've looked at the code (now lives in NodeGrantDatabaseStorage::write) and I can't see how it would "blow up". Given the age of this issue and lack of activity I'm going to close it for now.

  • Assigned to xjm
  • Status changed to Active 21 days ago
  • Comment 21 days ago →
    🇺🇸United States xjm

    The code is still essentially the same, so I should probably try to determine what I was talking about in my review of [#7173848-187]. The problem that I believe I was talking about was the following codepath:

    if (isset($grant['langcode'])) {
        $grant_languages = array($grant['langcode'] => language_load($grant['langcode']));
 }

    ...in the scenario where the Language module or the language in question was not available.

    language_load() not defined ➡️ fatal. This potential problem goes away with post-release Drupal 8 now that the language manager is always available.

    Language not currently available on site ➡️ an array [$langcode => NULL]. Then the foreach would set it to a null langcode. That's the part I was asking for test coverage for, because this could potentially cause strange things for site owner expectations if a language is disabled and then re-enabled, for exampled.

  • Comment 21 days ago →
    🇺🇸United States xjm

    Was going to assign it to myself to explain later but then ended up explaining it now.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024