As part of making language support expand wider in Drupal core, we are moving language related code to their respective modules instead of locale/language module implementing it for them on their behalf. This mostly happened for path module in #1236680: Move path language settings from Locale to Path module and is further worked on for path and node modules in #1414314: Make node and path depend on language module only for language support, get rid of locale_language_name and #540294: Move node language settings from Locale to Node module.

The attached patch does the very simple code change to integrate language support in comment module. With this moving out of locale module and being only dependent on node settings, it will make comment support language without locale module being present, which is a major goal for Drupal 8.

The mentioned issues are a set of issues that are going to have their full effect when all land.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, untangle-comment-from-locale.patch, failed testing.

tstoeckler’s picture

+++ b/core/modules/comment/comment.module
@@ -1833,11 +1833,22 @@ function comment_form($form, &$form_state, $comment) {
+  if ($comment_langcode == LANGUAGE_NONE && variable_get('language_content_type_' . $node->type, 0)) {

I know this is how it was before, but can we wrap the first check in parens, please? I hate having to look up/remember operator precedence (== vs. &&).

+++ b/core/modules/comment/comment.module
@@ -1833,11 +1833,22 @@ function comment_form($form, &$form_state, $comment) {
+  )

Missing the semi-colon here, as you've probably noticed.

-24 days to next Drupal core point release.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
2.45 KB

Right! Fixes attached. Thanks for the review.

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Wow, your patching speed is insane! :)
Anyway, this is RTBC (if it comes back green).

Status: Reviewed & tested by the community » Needs work
Issue tags: -D8MI

The last submitted patch, untangle-comment-from-locale-3.patch, failed testing.

tstoeckler’s picture

Status: Needs work » Needs review
Issue tags: +D8MI
Tor Arne Thune’s picture

Status: Needs review » Needs work

One of the test failures is caused by testHtmlEntitiesSample() that was just committed: #61456: Aggregator titles display quotes and other characters with HTML entity equivalents badly (write tests).

Gábor Hojtsy’s picture

Status: Needs work » Needs review
Gábor Hojtsy’s picture

Issue tags: +sprint

Tagging as current focus.

Tor Arne Thune’s picture

Status: Needs review » Reviewed & tested by the community

I also manually tested this patch and the language inheritance of comments perform the same with or without the patch applied.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

I agree that locale.module should not have comment module specific code. Modules should be written with multi-lingual in mind.

Committed this patch to 8.x.

Gábor Hojtsy’s picture

Issue tags: -sprint

Landed.

Gábor Hojtsy’s picture

Issue tags: +language-content

Tagging for the content handling leg of D8MI.

Status: Fixed » Closed (fixed)

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