Problem/Motivation

Drupal 7 had configurable languages which could be either enabled or disabled or removed. Drupal 8 has no feature to disable/enable languages so languages are either added or not added at all. Disabled languages had so narrow and confusing use cases that it was pointless to attempt to support them.

The user interface however is still referring to enabled languages at various places.

Proposed resolution

Do not refer to enabled/disabled languages anymore on the user interface. This is a non-existent feature. Languages are now added/removed, we should use that terminology.

Remaining tasks

User interface changes

Interface text will be correct and up to date with the Drupal 8 behaviour.

API changes

None.

CommentFileSizeAuthor
#26 interdiff-2240463-23-26.txt7.35 KBrobertdbailey
#26 2240463-enabled-26.patch19.37 KBrobertdbailey
#23 2240463-enabled-23.patch12.02 KBbalagan
#21 2240463-enabled-21.patch10.68 KBbalagan
#12 2240463-enabled-12.patch12.06 KBbalagan
#1 2240463-enabled-1.patch806 bytesbalagan

Comments

balagan’s picture

balagan commented
StatusFileSize
new 2240463-enabled-1.patch806 bytes

Added a patch that changes "languages enabled" to "languages added".

balagan’s picture

balagan commented
Assigned: balagan » Unassigned
Status: Active » Needs review
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue tags: +D8MI, +language-base, +sprint
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Postponed (maintainer needs more info)

It would be great to look for where if we talk about enabled languages elsewhere. I think we probably are. Did you look?

balagan’s picture

balagan commented

Yes, I was working in another issue: https://drupal.org/node/1870746, and there I was convinced by YesCT that I should use the word enabled.
Ah, I thought you mean "added". I am getting confused myself :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

The question is if "enabled languages" is used elsewhere, let's not have an issue for each instance to fix :) Can we fix more in this issue or is this the only left?

balagan’s picture

balagan commented

I did some search and came up with the following:

"enabled language":

\core\modules\language\config\tour.tour.language-add.yml
\core\modules\language\config\tour.tour.language-edit.yml
\core\modules\content_translation\lib\Drupal\content_translation\Tests\ContentTranslationTestBase.php
\core\modules\views\lib\Drupal\views\Tests\Entity\RowEntityRenderersTest.php

in comments:
\core\lib\Drupal\Component\Utility\UserAgent.php
\sites\default\default.settings.php
\core\modules\language\lib\Drupal\language\Plugin\LanguageNegotiation\LanguageNegotiationUrl.php

"languages enabled":
\core\modules\search\lib\Drupal\search\Tests\SearchLanguageTest.php

"language enable":
\core\lib\Drupal\Core\Language\LanguageManagerInterface.php

in comments:
\core\modules\system\lib\Drupal\system\Tests\Module\DependencyTest.php
\core\modules\language\lib\Drupal\language\Tests\LanguageUILanguageNegotiationTest.php
\core\modules\user\lib\Drupal\user\AccountFormController.php

"enable language":

in comments:
\core\modules\node\lib\Drupal\node\NodeListBuilder.php (maybe "enable language column" is correct, but "multiple languages are enabled" is definitely not.

"languages are enabled":
in comments:
\core\modules\content_translation\content_translation.install

"enable a language":
no occurance

Any other idea to search for?

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Title: Text says languages enabled, whereas they are not enabled but added » Fix text: languages are not "enabled/disabled" anymore, they are added/removed
Status: Postponed (maintainer needs more info) » Needs review

Sounds like a good comprehensive list to fix. Also look for disabled IMHO. :)

balagan’s picture

balagan commented

"enabling a language": I think it is a false positive: \core\modules\language\lib\Drupal\language\Tests\LanguageListModuleInstallTest.php

"disabled language":
\core\modules\language\lib\Drupal\language\Tests\LanguageListTest.php

"languages disabl", "languages are disabl", "disable a language", "disabling a language":
no occurance

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work
balagan’s picture

balagan commented
Assigned: Unassigned » balagan
balagan’s picture

balagan commented
StatusFileSize
new 2240463-enabled-12.patch12.06 KB

I have changed the "enabled" and "disabled" etc. to "added" and "removed" respectively.

balagan’s picture

balagan commented
Assigned: balagan » Unassigned
Status: Needs work » Needs review
balagan’s picture

balagan commented

The following two files mentoned above are not yet committed, so I have introduced these changes in the its corresponding issue:
\core\modules\language\config\tour.tour.language-add.yml
\core\modules\language\config\tour.tour.language-edit.yml

yesct’s picture

yesct commented
Issue summary: View changes
Issue tags: +Novice

this could be a novice review.

yesct’s picture

yesct commented
Issue summary: View changes
Related issues: +#2017471: Multilingual tour for language section

using [#NNNNN] to link to the related issue in the issue summary, and adding related issue.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes

Updated issue summary.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

12: 2240463-enabled-12.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 12: 2240463-enabled-12.patch, failed testing.

balagan’s picture

balagan commented
StatusFileSize
new 2240463-enabled-21.patch10.68 KB

Patch rerolled.

balagan’s picture

balagan commented
Status: Needs work » Needs review
balagan’s picture

balagan commented
StatusFileSize
new 2240463-enabled-23.patch12.02 KB

Of course I have forgot to save some files I have changed. Uploaded the file again.

holly.ross.drupal’s picture

holly.ross.drupal commented
Assigned: Unassigned » holly.ross.drupal
holly.ross.drupal’s picture

holly.ross.drupal commented

Reviewed patch in comment #23. All instances of "enable" look to have been updated to "add" appropriately. However, I was thrown by this change:

-     $this->assertRaw(t('The %language (%langcode) language has been removed.', $t_args), 'Disabled language has been removed.');
+    $this->assertRaw(t('The %language (%langcode) language has been removed.', $t_args), 'The French language has been removed.');

I could not see why the updated language should reflect French specifically.

Checked coding standard for indents, spaces around operators, and line length. All looked good. The other standards are still hard for me to see.

robertdbailey’s picture

robertdbailey commented
StatusFileSize
new 2240463-enabled-26.patch19.37 KB
new interdiff-2240463-23-26.txt7.35 KB

Good post, @holly.ross.drupal, and the hard-coded French reference seemed out of place to me at first, too. It appears that this is a test case, which references French as the language being tested. So, I think it makes sense that the assertion mentions the language.

Hi @balagan, I double checked the code for additional references to enable/disable languages. There were a few more references to "enable" that could be changed to "add", so I added them to the patch (along with an interdiff for reference).

balagan’s picture

balagan commented

Thanks @robertdbailey for the updates, it seems I have still missed some.
And yes, I have included "French" in the text, because that is a test, and this is what precedes that string:
$t_args = array('%language' => 'French', '%langcode' => 'fr');
So I thought I might just say exactly what is happening.
I took a look at the patch in comment #26, and all seems fine. I am not sure if I myself can change the status to RTBC, since I wrote the first half of the patch, but since it concerns a lot of files it would make sense to get it reviewed quickly, before another reroll will be needed :).

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Yay, thanks! Looks good to me too.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Oops. Thought I committed this on Friday. Doing so now. :)

Committed and pushed to 8.x. Thanks!

  • Commit e4ae152 on 8.x by webchick: 
    Issue #2240463 by robertdbailey, balagan, holly.ross.drupal: Fix text:...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -sprint

Yay, thanks!

Status: Fixed » Closed (fixed)

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