Follow up for #1831636: "Translation is not supported if language is always one of: Not specified, Not applicable, Multiple" reword to: "...."

Problem/Motivation

While trying to word the error message that is the result of incompatible configuration, @webchick said making the error message better is A Good Thing. But, that really it would be better to have the interface not allow picking wrong choices.

Steps to reproduce

  1. Install drupal (8.x head)
  2. Under Extend, install/enable module: Content Translation
  3. (optional, but makes sense to) Add another language, Under configuration, regional and languages, languages, add language, at admin/config/regional/language
  4. Edit the content type Article, at admin/structure/types/manage/article
  5. select the Language settings to see the dropdown and checkboxes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because there is no bug.
Issue priority Normal because it is isolated and not across systems.
Unfrozen changes NOT Unfrozen only changes.
Prioritized changes The main goal of this issue is usability.
Disruption NOT Disruptive for core/contributed and custom modules/themes because it will not require a BC break/deprecation/internal refactoring/widespread changes.

Allowed in the drupal 8 beta because it is a prioritized change where the usability improvement has an impact greater than the disruption.

Proposed resolution

This needs discussion, but a start suggestion is to not allow choosing the system languages (only showing specific ones?) when translation is already enabled and the lang select is hidden.

Remaining tasks

  • explicitly write down the combinations of flows that goes through when to hide what and what to allow to be selected
  • maybe mark up an example of one of the flows
  • get screenshots
  • get ui review
  • revisit proposed solution
  • implement
  • manually test
  • write tests
  • review code
  • etc

User interface changes

This is a ui issue. See above.

API changes

No api changes anticipated.

Files: 
CommentFileSizeAuthor
#73 1834266-73.patch7.88 KBrodrigoaguilera
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,323 pass(es), 0 fail(s), and 52 exception(s). View
#71 1834266-71.patch7.84 KBrpayanm
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,937 pass(es), 0 fail(s), and 36 exception(s). View

Comments

plach’s picture

While doing this ensure that it is still possibile to hide the language selector and enable translation if a valid default language is selected.

e0ipso’s picture

I have identified the following workflow:

Background:

  1. Content translation & Locale are enabled.
  2. There is, at least, one content type to work with.

Case 1:

  1. Edit your contente type (Article in my case).
  2. Under the Language settings vertical tab select:
    • Uncheck checkbox Show language selector on create and edit pages.
    • Check checkbox Enable translation.
    • Select - Not specified - in Default language.
  3. Save the content type.

A validation error forbids you to save the content type.
Case 1 screenshot

Case 2:

This case is exactly the same one as Case 1 but selecting - Not applicable - as the Default language.
Case 2 screenshot

Case 3:

This case is exactly the same one as Case 1 but checking Show language selector on create and edit pages. In this case the validation error does not exist and therefore the user can successfully save the content type.
Case 3 screenshot

Case 4:

This case is exactly the same one as Case 1 but selecting English (or any other active language) as the Default language. In this case the validation error does not exist and therefore the user can successfully save the content type.
Case 4 screenshot

e0ipso’s picture

Assigned: Unassigned » e0ipso

Assigning to myself in the context of core menthoring (with xjm).

Writing a patch to test.

Gábor Hojtsy’s picture

The cases explained look all good and correct.

Gábor Hojtsy’s picture

Also, same interaction happens on the content language configuration screen (in regional and languages), although for all entities altogether.

e0ipso’s picture

I would want to test the same behavior when selecting a locked language, but I do not know what those are or how to get one. Any thoughts?

Gábor Hojtsy’s picture

Not specified and not applicable are the two locked (non-configurable, non-removable) languages in core. Contrib can add more, there is no UI for these which is the nature on the non-configurability. :)

e0ipso’s picture

Thanks!

I was thinking on hiding 'und', 'mul' and 'zxx' from the select but if contrib can add locked languages of their own this means the list should be retrieved in an Ajax call where the locked property can be tested.

Another option could be testing the language name against the regexp /^- .* -$/ in the JS side. Can we rely on a regexp like that, won't we be missing locked languages or having false alarms?

Other options include changing the form structure or/and markup to identify these locked languages, I'm not taking those into account for now.

Gábor Hojtsy’s picture

Well, we could generate some more metadata alongside the select box for the JS to use. I personally believe dynamically removing items from the select box when you check another checkbox might be unexpected and especially without explanation pretty surprising.

e0ipso’s picture

FileSize
4.11 KB
PASSED: [[SimpleTest]]: [MySQL] 49,350 pass(es). View
56.27 KB
60.36 KB
180.86 KB

The following patch detects locked language in the frontend part using a regex (like the one #8).

The patch performs the detection for each select individually, taking into account different default values allowed per entity bundle.

The same behavior is applied in the vocabulary edit form & node type edit form.

Taking #9 into account, we could evolve this patch to provide a hidden list of locked languages within the form to detect locked languages more accurately (instead of the regular expresion).

Entity translation settings
Entity translation settings

Taxonomy settings 1
Taxonomy settings 1

Taxonomy settings 2
Taxonomy settings 2

e0ipso’s picture

Status: Active » Needs review
FileSize
4.11 KB
PASSED: [[SimpleTest]]: [MySQL] 49,377 pass(es). View

Changing status.

Gábor Hojtsy’s picture

Issue tags: +Needs usability review

The disappearing/appearing description looks interesting. Maybe we should reserve space for it in the form, so it does not make the form jump around when clicked. However, before going deeper into this, it would probably be best to get some usability feedback. Tagging for that, so Bojhan et. al. can take a look.

plach’s picture

Not sure I interpreted the screenshots correctly but locked languages should be hidden only when the bundle is translatable and the language selector is not shown.

e0ipso’s picture

Status: Needs review » Needs work

plach is right in #13. This behavior should only be triggered when the bundle is translatable.

plach’s picture

Title: configuring translation (hiding lang select, selecting system or specific language) allow site builder to only make valid choice » Force site builders to make only valid choices when configuring entity default language with translation enabled

Improved title

e0ipso’s picture

The new patch takes into account the comments by plach in #13.

Screenshots are from node type edit form:

Enabled + hidden (should hide locked languages)
Enabled + hidden

Enabled + shown (should not hide locked languages)
Enabled + shown

Disabled + shown (should not hide locked languages)
Disabled + shown

Disabled + hidden (should not hide locked languages)
Disabled + hidden

e0ipso’s picture

Status: Needs work » Needs review

I forgot to update the status … again :-P

e0ipso’s picture

Issue tags: +Needs manual testing

See contributor task doc on manual testing: http://drupal.org/node/1489010

jsbalsera’s picture

FileSize
31.4 KB
33.82 KB
29.74 KB
28.33 KB
7.04 KB
PASSED: [[SimpleTest]]: [MySQL] 53,107 pass(es). View

Rerolled patch

It seems to work as described in #16 in Linux with Google Chrome Version 25.0.1364.97

Not message and not hidden options:
Disabled + Hidden:
 Disabled + Hidden

Disabled + Shown:
Disabled + Shown

Enabled + Shown:
Enabled + Shown

Hidden options and explanation
Enabled + Hidden.png

e0ipso’s picture

Issue tags: +SprintWeekend2013
e0ipso’s picture

Assigned: e0ipso » Unassigned

Unassign while waiting for RTBC.

Gábor Hojtsy’s picture

Status: Needs review » Needs work
+++ b/core/modules/translation_entity/translation_entity.admin.incundefined
@@ -136,9 +136,21 @@ function _translation_entity_preprocess_language_content_settings_table(&$variab
+    $rows[$bundle]['data'][2]['data']['language']['language_show']['#description'] = t('Some default language options are hidden if the language selector is not shown.');

+++ b/core/modules/translation_entity/translation_entity.moduleundefined
@@ -922,6 +922,20 @@ function translation_entity_language_configuration_element_process(array $elemen
+    $element['language_show']['#description'] = t('Some default language options are hidden if the language selector is not shown and translation is enabled.');

It looks a bit odd to have this description on the checkbox for showing the language selector. It talks about options in the language dropdown and therefore describes what happened to another field, not this one.

+++ b/core/modules/translation_entity/translation_entity.admin.jsundefined
@@ -103,4 +103,67 @@ Drupal.behaviors.translationEntity = {
+/**
+ * Removes unusable locked languages when not showing language selector.
+ * This behavior gets attached on the relevant entity edition form and
+ * the translation entity summary form.
+ */

As per Drupal code style there needs to be an empty line after the first line.

+++ b/core/modules/translation_entity/translation_entity.admin.jsundefined
@@ -103,4 +103,67 @@ Drupal.behaviors.translationEntity = {
+         var $unlockedOptions = $originalOptions.filter(function (index) { return !/^- .* -$/.test(this.text); });

Was this cross-checked to be always used as is - -?

+++ b/core/modules/translation_entity/translation_entity.admin.jsundefined
@@ -103,4 +103,67 @@ Drupal.behaviors.translationEntity = {
+             $checkbox.siblings('.description').css('opacity', 0);
+           }
+           else {
+             $languageSelector.html($unlockedOptions);
+             $checkbox.siblings('.description').css('opacity', 1);
+           }
+         }
+         else {
+           $languageSelector.html($originalOptions);
+           $checkbox.siblings('.description').css('opacity', 0);

Why are you using opacity to hide the element? Looks strange to me.

Also seems like if I had an option selected and that option vanished, I'm not alerted at all visually that my settings will largely change. At least if the process removed the option I had selected before, it should let me know with a yellow box appearing there or something IMHO. Now that given this adjustment the server side will not complain anymore.

e0ipso’s picture

FileSize
7.98 KB
7.65 KB
PASSED: [[SimpleTest]]: [MySQL] 53,281 pass(es). View
80.25 KB

Was this cross-checked to be always used as is - -?

Re-checked. It appears on:

core/modules//language/language.module:256
core/modules//language/language.module:285
core/modules//node/node.admin.inc:116
Why are you using opacity to hide the element? Looks strange to me.

This was done because of your suggestion in #12, but I agree that is better hiding and showing it.

The disappearing/appearing description looks interesting. Maybe we should reserve space for it in the form, so it does not make the form jump around when clicked.

I added a standard warning message if the selected language is changed and moved the description to the language dropdown.

Screenshot_3_27_13_9_43-2.png

e0ipso’s picture

Assigned: Unassigned » e0ipso
Status: Needs work » Needs review
YesCT’s picture

Status: Needs review » Needs work

on admin/config/regional/content-language
when checking the show language select, it changes the drop down for default language to show (not hide) the not specified and not applicable choices.

but when checking translatable (which has the effect of checking the show language select) the drop down values do not change. they need to also show (not hide) the choices.

e0ipso’s picture

Status: Needs review » Needs work

Edit (posted twice by mistake)

e0ipso’s picture

Status: Needs work » Needs review

@YesCT according to plach on #13

[...] locked languages should be hidden only when the bundle is translatable and the language selector is not shown.

As I see it, if the language selector is shown then the user will be forced to make a valid choice on content creation, and locked languages may be the default initial choice.

Is this assumption correct?

Bojhan’s picture

Status: Needs work » Needs review
Issue tags: -Needs usability review

Wondering, could these messages be a little bit more friendlier. Less focused on the user, and more on the system needing something.

- "You default language has been changed. You need to show the language selector to use a locked language."
+ You default language has been changed. To use a locked language, the language selector needs to be shown"

Also not sure about having "Some" in a message, that is quite ambiguous.

e0ipso’s picture

FileSize
7.64 KB
PASSED: [[SimpleTest]]: [MySQL] 54,850 pass(es). View
792 bytes

Chaged wording according to #28.

YesCT’s picture

FileSize
5.34 KB
7.65 KB
PASSED: [[SimpleTest]]: [MySQL] 55,560 pass(es). View

just some grammer, line length (must be 80 chars or less for comments), and newlines for comment standards.

I'll text next.

YesCT’s picture

Status: Needs review » Needs work
FileSize
266.45 KB

Reminder, when testing, this needs to be tested on admin/config/regional/content-language too.

This patch removes languages from the drop down of *all* the language dropdowns, not just the table that has translation enabled and not showing the lang selector.

So when saving that page, changes are saved for a bunch of types that it was not intended to change.

http://screencast.com/t/8Z5qzlocixvd

content_languages_settings_page.png

YesCT’s picture

FileSize
146.04 KB
313.4 KB

The "some" languages that are not shown are also known as locked in the code, but I dont think we should use that word in the UI. They are also known as system languages at some point.. and then we just listed all of them too: Not specified, Not applicable

the link to "Explanation of the language options is found on the languages list page."
goes to admin/config/regional/language
Which used to list the locked/system languages Not specified and Not applicable, but we decided that showing them there to reorder them was just unneeded. So they are not "explained" there.

"Some default language options are hidden if the language selector is not shown."
to:
"Not specified, Not applicable, default language options are hidden if the language selector is not shown."

That list can grow if contrib adds other locked languages.

different_message.png

not_hidden.png

YesCT’s picture

+++ b/core/modules/translation_entity/translation_entity.admin.jsundefined
@@ -103,4 +103,74 @@ Drupal.behaviors.translationEntity = {
+     return '<div class="messages warning">' + Drupal.t('Your default language has been changed. To use a locked language, the language selector needs to be shown.') + '</div>';

Note this didn't show for me while I was testing.

Anyway, it's not the default language that is changed... it the choices shown that are changed.

[edit: removed wrong code chunk]

e0ipso’s picture

FileSize
7.96 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/translation_entity/translation_entity.admin.inc. View

I re-rolled the patch and improved the warning message.

@YesCT, please make sure you have one locked language selected when hiding language selector (or enabling translation) to make the warning message to appear.

e0ipso’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-Allow-only-valid-choices-1834266-34.patch, failed testing.

jsbalsera’s picture

FileSize
7.98 KB
PASSED: [[SimpleTest]]: [MySQL] 57,410 pass(es). View

Re-rolled the patch and fixed a php error.

e0ipso’s picture

Status: Needs work » Needs review
jsbalsera’s picture

Status: Needs review » Needs work
FileSize
698 bytes

Adding also the interdiff

jsbalsera’s picture

Status: Needs work » Needs review

Sorry, I answered without noticing that e0ipso had already answered changing the status... so I changed the status by mistake. I change it again now.

YesCT’s picture

Assigned: e0ipso » Unassigned
Status: Needs review » Needs work
Issue tags: +Novice

Just a few picky docs and coding standards things.

+++ b/core/modules/translation_entity/translation_entity.admin.incundefined
@@ -161,6 +171,7 @@ function _translation_entity_preprocess_language_content_settings_table(&$variab
+

unrelated change. lets put this back. it will keep the patch smaller and easier to review.

+++ b/core/modules/translation_entity/translation_entity.admin.jsundefined
@@ -103,4 +103,77 @@ Drupal.behaviors.translationEntity = {
+        // Store the data about original options in the language selector
+        // Do it only once to avoid losing options

Should be sentence (add a period at the end of each sentence.) Also, wrap them together (but limit to 80 chars).

----
also, good catch in #39.

Next: Go ahead and make a new patch with these changes and that one. (This might be a good novice task, to do just that bit of improvement.)

---

needs still someone to try it. (I'll do that next)
Also, someone who knows js to see if the approach is a good one, and do a review.

YesCT’s picture

I tried this in chrome, firefox, safari.. (all ok)
and IE via a trial to browserstack.com I noticed something strange there.
Seems like the red error message does not she when have a locked language and then pick to hide.
Also, the extra help text does not disappear when clicking the checkbox to show the language select.

this needs someone with IE to actually try it.

Usability question, should the red message go away, when things are ok (when the help text goes away)? It stays.

screenshots:

had locked language selected when unchecked show selector (hid selector)

had-locked-when-hid.png

just hid selector, lock languages are hidden, and the extra helptext shows great

just-hide-selector-getextrahelptext-andlockedarehidden.png

firefox ok

firefox-ok.png

safari ok

safari-ok.png

IE. Maybe needs work. Needs try in IE natively.

ie-browserstack.png

Gábor Hojtsy’s picture

Component: translation_entity.module » content_translation.module
jair’s picture

Issue tags: +Needs reroll
luismagr’s picture

Status: Needs work » Needs review
FileSize
8.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,762 pass(es). View

Re-rolled the patch #37 and run tests

e0ipso’s picture

FileSize
8.05 KB
PASSED: [[SimpleTest]]: [MySQL] 58,712 pass(es). View

There was a problem with the previous re-roll.

120c120
< +    $element['#attached']['library'][] = array('translation_entity', 'drupal.translation_entity.admin');
---
> +    $element['#attached']['library'][] = array('content_translation', 'drupal.content_translation.admin');

Should pass tests.

e0ipso’s picture

Assigned: Unassigned » e0ipso
tim.plunkett’s picture

Issue tags: -Needs reroll

Removing tags

YesCT’s picture

Issue summary: View changes

updated issue summary with steps to reporduce

YesCT’s picture

Issue summary: View changes

added adding a language to the steps to reproduce

eshta’s picture

Assigned: e0ipso » eshta

Working on during a drupal mentoring session.

eshta’s picture

FileSize
8.08 KB
FAILED: [[SimpleTest]]: [MySQL] 59,891 pass(es), 1 fail(s), and 4 exception(s). View

Re-rolled patch to current Drupal 8 version.

Status: Needs review » Needs work

The last submitted patch, 52: drupal-allow-only-valid-choices-1834266-52.patch, failed testing.

eshta’s picture

Status: Needs work » Needs review
FileSize
8.07 KB
PASSED: [[SimpleTest]]: [MySQL] 59,885 pass(es). View

Re-rolled.

eshta’s picture

Assigned: eshta » Unassigned
hpz’s picture

Status: Needs review » Needs work

I did tests on firefox:
When enabling translation with a locked language it works correctly and the message

"To use a locked language, with translation enabled, the language selector needs to be shown."

is shown above correctly.
But the the message didn't disappear when enabling the language selector or when doing any changes. It would be nice if the message disappears again.

Further, I noticed, when setting e.g. Default language to 'Current Interface language' and uncheck the 'Show language selector', the Default language setting changes in my case to 'German'. Re-checking 'Show language selector' the Default language setting jumping to '- not applicable -'.

hpz’s picture

Just did a test on IE:
the second note, 'Default Language' setting stays correctly when un-checking and re-checking the 'Show language selector' box. So okay with IE.

naveko’s picture

The patch in #54 doesn't apply.
I got an error notice when trying to save the content type 'article' with these settings:
- Default language: not specified
- Show language selector on create and edit pages: off
- Enable translation: on.
The error notice:
Notice: Undefined index: und in content_translation_language_configuration_element_validate() (line 847 of core/modules/content_translation/content_translation.module).

naveko’s picture

FileSize
7.89 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,731 pass(es), 0 fail(s), and 18 exception(s). View

Uploading a new patch that applies to the current state of core.

heddn’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 59: drupal-allow-only-valid-choices-1834266.59.patch, failed testing.

rodrigoaguilera’s picture

Status: Needs work » Needs review
FileSize
7.88 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,728 pass(es). View

Fix to pass tests by doing the correct attach

rodrigoaguilera’s picture

FileSize
935 bytes

add interdiff for my last patch

Status: Needs review » Needs work

The last submitted patch, 62: drupal-allow-only-valid-choices-1834266.62.patch, failed testing.

jsbalsera’s picture

jsbalsera’s picture

Status: Needs work » Needs review
YesCT’s picture

YesCT’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll
rpayanm’s picture

Status: Needs work » Needs review
FileSize
7.81 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,238 pass(es). View
YesCT’s picture

Status: Needs review » Needs work

needs reroll again.

rpayanm’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
7.84 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,937 pass(es), 0 fail(s), and 36 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 71: 1834266-71.patch, failed testing.

rodrigoaguilera’s picture

Status: Needs work » Needs review
FileSize
7.88 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,323 pass(es), 0 fail(s), and 52 exception(s). View

Patch rerolled and some linting.

Status: Needs review » Needs work

The last submitted patch, 73: 1834266-73.patch, failed testing.

The last submitted patch, 73: 1834266-73.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.