Problem/Motivation
Follow-up to #2584603-48: PHP exception on manage fields after enabling Configuration Translation
we cannot actually prevent having config files associated to the same mapper in different languages (which prevents translation). If we only add the file to the mapper if the langcodes match we have a sort of "silent failing" in that (in this case) you cannot translate the node type label (because the respective file is not added to the mapper) but you get absolutely no feedback that something is wrong or why it is. Bad.
Step to reproduce:
1- Install a drupal 8 in Spanish.
2- In "admin/config/regional/language/add" add Inglés language.
3- Go to "/en/admin/modules" install "Configuration Translation" and "Content Translation" modules.
4- Go to "/en/admin/structure/types/manage/page/fields", click in "Add field", in "Re-use an existing field', select "Image: Field image", "Save and continue".
5- Bump!
RuntimeException: A config mapper can only contain configuration for a single language. in Drupal\config_translation\ConfigNamesMapper->getLangcode() (line 398 of core/modules/config_translation/src/ConfigNamesMapper.php).
Drupal\config_translation\Access\ConfigTranslationOverviewAccess->access(Object, Object)
call_user_func_array(Array, Array)
Drupal\Core\Access\AccessManager->performCheck('config_translation.access.overview', Object)
Drupal\Core\Access\AccessManager->check(Object, Object, NULL, 1)
Drupal\Core\Access\AccessManager->checkNamedRoute('entity.field_config.config_translation_overview.node', Array, Object, 1)
Drupal\Core\Menu\LocalTaskManager->getTasksBuild('entity.field_config.node_field_edit_form', Object)
Drupal\Core\Menu\LocalTaskManager->getLocalTasks('entity.field_config.node_field_edit_form', 0)
Drupal\Core\Menu\Plugin\Block\LocalTasksBlock->build()
Drupal\block\BlockViewBuilder::preRender(Array)
call_user_func('Drupal\block\BlockViewBuilder::preRender', Array)
Drupal\Core\Render\Renderer->doRender(Array)
Drupal\Core\Render\Renderer->doRender(Array, )
Drupal\Core\Render\Renderer->render(Array)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1)
__TwigTemplate_0f04497e0607dbb55ae785558deed68eacde76e3989f8c8e0779ccf1b6af549c->doDisplay(Array, Array)
Twig_Template->displayWithErrorHandling(Array, Array)
Twig_Template->display(Array)
Twig_Template->render(Array)
twig_render_template('core/themes/seven/templates/page.html.twig', Array)
Drupal\Core\Theme\ThemeManager->render('page', Array)
Drupal\Core\Render\Renderer->doRender(Array, )
Drupal\Core\Render\Renderer->render(Array)
Drupal\Core\Template\TwigExtension->escapeFilter(Object, Array, 'html', NULL, 1)
__TwigTemplate_16436f2456eb6e9f1287d658a3b71677eb9755e9eb4796bd9e27a1209ebeaf7f->doDisplay(Array, Array)
Twig_Template->displayWithErrorHandling(Array, Array)
Twig_Template->display(Array)
Twig_Template->render(Array)
twig_render_template('core/themes/classy/templates/layout/html.html.twig', Array)
Drupal\Core\Theme\ThemeManager->render('html', Array)
Drupal\Core\Render\Renderer->doRender(Array, )
Drupal\Core\Render\Renderer->render(Array)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1)
Stack\StackedHttpKernel->handle(Object, 1, 1)
Drupal\Core\DrupalKernel->handle(Object)
Fields have two config objects,
the "Edit" tab is field.field... and
the "Field settings" the field.storage...
When installing in spanish, using standard profile, the field and storage config have langcode es.
After adding english, and enabling configuration translation module, and specifying en in the admin url, the field config (not storage) has the langcode en.
The translate tab, then errors because it expects all the config on the form to have the same source.
This is only a problem when config translation is enabled. (The field stuff is fine.)
Proposed resolution
We should "fail gracefully" by catching the exception in the UI and notifying the user about this in some way.
- Still show the translate tabs and everything (and make sure that no fatals bubble up, i.e. catch the exception)
- On the translation overview show a drupal_set_message(..., 'warning')
with something like
The configuration files related to the node type Article have different source language codes and thus cannot be translated:
or
"Warning, the field configuration and field settings configuration have different source languages, and will not be able to be translated."
- node.type.article: Spanish
- core.base_field_override.node.title.article: Hungarian
We could do other things as well (maybe followups), ideas could be:
- Show some suggestion on how to fix this (maybe link to d.o)
- (no) Fix this automatically somehow
- Present a UI to choose which langcode to set for all the config files
- Still allow to translate some of the config files for which the langcode matches (and still show a warning)
Remaining tasks
Task | Novice task? | Contributor instructions | Complete? |
---|---|---|---|
Create a patch | Instructions | ||
Add automated tests | Instructions | ||
Embed before and after screenshots in the issue summary | Novice | Instructions | |
Review patch to ensure that it fixes the issue, stays within scope, is properly documented, and follows coding standards | Instructions |
User interface changes
Adding a warning.
API changes
??
Data model changes
No.
Comment | File | Size | Author |
---|---|---|---|
#91 | Failqm.JPG | 62.22 KB | Benia |
#87 | fail.JPG | 88.43 KB | Benia |
#80 | 2595535-80.patch | 20.33 KB | catch |
#80 | interdiff.txt | 1.79 KB | catch |
#74 | 2595535-74-config-translation-langcode-exception.patch | 20.68 KB | tstoeckler |
Comments
Comment #2
tstoecklerUnless I'm mistaken, this is something the direction that @Gábor Hojtsy was proposing.
In addition to silently failing, this has the problem of not working for people for implement
hook_config_translation_info_alter()
, but only people usingConfigNamesMapper::getLangcode()
and I don't see how we could get around that.Comment #4
tstoecklerHere's something more along the lines of what I was thinking. Wording, etc. to be done, but it's a start.
Comment #5
tstoecklerComment #7
Gábor HojtsyThe proposed patch seems to let things go on thinking the source language is English even if technically we would not be able to tell what is even translated (source languages don't match) and we cannot display a transaltion form this way. I don't think its feasible to allow access to these pages. I would disallow access and display that message, so we don't attempt to go into displaying overview tables or translation forms if we cannot.
Comment #8
YesCT CreditAttribution: YesCT commentedcopying some information from issue closed duplicate of this
#2623908: A config mapper can only contain configuration for a single language
Comment #9
rpayanmI created the issue #2623908: A config mapper can only contain configuration for a single language and the "Step to reproduce". But today I tested the 8.0.1 release and the error was gone.
Comment #10
Codenator CreditAttribution: Codenator as a volunteer commentedI got error on Drupal 8.0.2 when add language and try edit Content type -> Article
edited: When uninstall Configuration Translation module error gone
Way this not critical? On my site this module , when enabled, block me from edit fields in Content type
Comment #11
rpayanmOh yes, this come back :(
Comment #12
Gábor Hojtsy@swentel closed #2584603: PHP exception on manage fields after enabling Configuration Translation (again) referencing this issue. If this issue is to solve the recurring problem, then this needs to be critical as well. Three reports from that issue:
Comment #13
swentel CreditAttribution: swentel as a volunteer commentedTook me a while to figure out you needed tabs to get the bug exposes in tests ..
Comment #15
swentel CreditAttribution: swentel as a volunteer commentedThis merges the patch from @tstoeckler in.
Now, this fixes the 're-using fields' action in the test I added in #13. However, upon clicking on the translate tabs, it still goes 500, as the test I've added also exposes (see interdiff for that).
Comment #17
maartendeblock CreditAttribution: maartendeblock at EntityOne commentedPatch #2595535-15: Show helpful message (do not fatal!) when configuration files have different source language codes and cannot be translated works for me on Drupal 8.0.2.
Comment #18
swentel CreditAttribution: swentel as a volunteer commentedThis also fixes the controller and showing the same message - inline.
Comment #19
vijaycs85Another update to reduce the duplication on exception message.
Comment #20
golddragon007 CreditAttribution: golddragon007 commentedHi, This patch fixed my issue. I use now 8.0.3.
My issue was:
I get this with the 8.0.3 D8 (and with the 8.0.2 too, I only updated from 8.0.2 to 8.0.3).
RuntimeException: A config mapper can only contain configuration for a single language. in Drupal\config_translation\ConfigNamesMapper->getLangcode() (line 400 of \core\modules\config_translation\src\ConfigNamesMapper.php).
Debug:
Watchdog log:
Error1: 'sk'
Error1: 'en'
Error2: array ( 0 => 'sk', 1 => 'en', )
Error3: array ( 0 => 'node.type.page', 1 => 'core.base_field_override.node.page.title', )
I tried to reproduce with a newly installed Drupal 8, but it wasn't successful.
Some basic information about the site:
There're 6 languages including with the English (which is not the default language, and basically it's not used, but it's not turned off). All internationalization module is turned on. I did a lot of things, before I detected this, so I don't know what caused this. :(
I get this when I go to /admin/structure/types/manage/page/fields . (editing the default two content type's [basic page, article] fields) With custom content type, I don't get any error, it works well.
Comment #21
EdizonTN CreditAttribution: EdizonTN commentedI confirm.
D8.0.3 + patch 2595535-19 is working perfectly now.
3 languages (CZ,SK,EN). Default SK.
Comment #22
EdizonTN CreditAttribution: EdizonTN commentedD8.0.4 + patch 2595535-19 is working also.
Comment #23
rpayanm2595535-19.patch in Drupal 8.0.4 works for me.
Comment #24
esolitosTesting #19 on an handful of sites on 8.0.4 and it seems to be working fine!
Comment #25
tstoecklerHere's an updated patch. The interdiff is with
-w
for readability's sake. I made the following changes:ConfigTranslationOverviewAccess
andConfigTranslationFormAccess
are set up, I had to use a try-catch block in both, which is not super nice, but I couldn't find any nicer way.drupal_set_message()
, like the form was doing before) and then show the rest of the overview in the normal way - except without the 'Add' and 'Edit' links for translations. I think it is more intuitive to show the screen that people are used to. If they then can make the warning disappear, the screen will still look the same (which is a good thing IMO), but with added Operations links.ConfigNamesMapper::getLangcodeFromConfig()
toConfigMapperInterface
. This is technically a BC-break, butConfigNamesMapper
is one of those classes that (sadly) does so much, that it's almost unthinkable to implementConfigMapperInterface
without extending it. Also I think a critical bug warrants this kind of super minor break. If deemed not possible for 8.1 and/or 8.0 we could of course revert that but I think then instead of introducing a check onConfigNamesMapper
(like the previous patch) we should just fetch the langcode using the config factory ourselves.getLangcodeFromConfig()
doesn't actually do anything super fancy it just seemed natural to split it out as there was a use-case for it andgetLangcode()
already hat the same logic.Comment #26
tstoecklerOops, forgot to attach the screenshot of the translation overview that I made with the patch applied.
Comment #28
tstoecklerFixed the test fails. I had updated the warning message to be a little bit more clear (IMO).
Using
und
as the language code changes the label for the source language from Unknown to None (see the screenshot in #26), which I think it slightly less clear, butund
(as in undetermined) is semantically the correct language code to use in this case. We could explicitly set the langcode toNULL
which would still avoid the notices and change the label back to Unknown but per the previous sentence I think that's not ideal.Comment #29
catchI think it's OK to use unspecified, however:
Should use LanguageInterface::LANGCODE_NOT_SPECIFIED instead of the string.
Only did a quick review, but nothing else stuck out here.
Comment #30
tstoecklerD'oh. Fixed that. Thanks!
Comment #31
vijaycs85Patch looks good and applies to 8.0.x. +1 to RTBC.
Comment #32
igasi CreditAttribution: igasi as a volunteer commentedWorks fine for me. Thanks a lot.
Comment #33
Kristen PolThanks for the patch! I've reviewed the code mainly for syntax/style.
Nitpick: More than 80 characters.
Nitpick: "use case" appears to be more common than "use-case".
Should this be $mapper->getConfigNames()? (uppercase C)
Incomplete comment.
t() functions missing, e.g. t('Save field settings')?
t() missing, e.g. t('Save configuration')?
t() missing, e.g. t('Save and continue')?
t() missing? I see core code use t() for assertText in some cases and not others.
Comment #34
tstoecklerThanks a lot for the review @Kristen Pol!
Fixed 1.-4. Not sure about 5.-8., I generally avoid t() completely in tests, but I'll let comitters decide that, so I'm providing an extra diff (on top of the patch) for the t() changes, in case it's deemed necessary.
Comment #35
Kristen PolThanks for the changes. I've reviewed them and they look good. I don't know about using t() in tests or not. I see both in core test code so I'm not sure which is "correct". It would be good to know if that is documented somewhere.
Comment #36
Kristen PolI have also tested the patch and it is working as expected. Marking RTBC. Thanks!
Comment #37
penyaskitoThose t() are needed as they depend on the UI strings. We don't use t() for assertion messages, etc, but we want tests to pass in case we end running them against a translated site.
Comment #38
penyaskitoForgot link to docs as @kristen_pol requested: https://www.drupal.org/simpletest-tutorial-drupal7#t
Comment #39
tstoecklerI'm fine with adding the
t()
, butis not true. The test code is actually executed from within the child site so it will always be English, even if the parent site is not - unless we explicitly change the default language and import translations earlier in the test.
Comment #40
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI discussed this with @xjm and @Cottser, and we agreed this is Critical, since it's a PHP fatal error that's not limited to rare circumstances.
Comment #41
Kristen Pol@penyaskito - Thanks for the documentation of using t() for tests.
It appears that adding t() for the tests is all that is left for this patch but @tstoeckler added some feedback in #39 that no one has explicitly addressed yet.
Comment #42
catchPer #39 I don't think we need the t() calls here at all in the tests.
The rest of the patch here looks fine to me, so moving to RTBC.
Comment #43
catchGiving @alexpott a chance to look at this before it goes in.
Comment #44
alexpottIs there test coverage for this?
Why are we making this change - I don't think we should be adding this protected property - it is only used in this method - adding it here means that PHP couldn't garbage collect it because the reference will exist until the access check service is destroyed.
We need to update the interface docs for this exception
Is this tested? I can't see where.
Not used
Is there test coverage for this?
Comment #45
tstoecklerComment #46
esolitosPatch from #34 didn't apply on 8.0.6, I rerolled the changes.
Comment #47
swentel CreditAttribution: swentel as a volunteer commentedthis should be rolled against 8.1.x, 8.0.x is out of support soon.
Comment #48
esolitosThe patch from #46 applies on 8.1.x as for today.
I haven't tested the functionality itself 'tho.
Comment #49
esolitosTriggering the bot for testing the new patch on 8.1.x
Comment #50
klausithis looks a bit obscure. Can you use "return AccessResult::forbidden()->inheritCacheability($base_access);" instead?
Hm, we cannot introduce new methods on interfaces like that since D8 is already released. Either you mark this interface as @internal, so that people know that this interface is fake and not actually an interface because we change it all the time - or you create a new interface that a ConfigMapper can optionally implement (LangcodeFromConfigInterface?).
must use $§this->t()
Comment #51
alvar0hurtad0I'm not really sure about second topic on #50, but I think this solves the other two topics.
Comment #52
alexpottI don't think this belongs here. Let's remove it.
I think we've covered adding methods to interfaces in other issues. I think contrib using ConfigMapperInterface on an object is extremely unlikey - it is more likely that contrib is using ConfigMapperInterface objects provided by core and these will not be broken by adding the method since they are just calling methods on it. See www.drupal.org/core/d8-bc-policy#interfaces
Not used - needs to be removed.
This needs to be translated. In french, for example, there always has to be a space before a colon. So
$items[] = $this->t('@config_name: @langcode', [... variables
Missing a comma
Need only one empty line here.
Don't need this anymore.
Not used. Need to be removed.
Needs a description.
Comment #53
stillworks CreditAttribution: stillworks commented#51 worked for me on Drupal 8.1
Comment #54
k4 CreditAttribution: k4 commentedIf you set up a multilingual site without configuration translation enabled and later enable the module you may not be able to edit content types any longer because of this issue.
This solves the problem (supposed the default language is english):
Export the config:
sync/*.yml
-> untranslated configsync/language/[langcode]/*.yml
-> translated configNow there are in
sync/*.yml
several configs which are notlangcode: en
.Change all config in
sync/*.yml
tolangcode: en
and import the config back to the site.Now you are able the edit the content types again. In future you have to edit content types only in the english admin interface so that the config does not get mixed up again.
The same should work with a non english default languages, but I have not tested this.
---
To help to prevent this issue, perhaps drupal should warn you, if you try to save a content type configuration with different languages?
Comment #55
tstoecklerFixed #52.
Comment #56
tstoecklerComment #57
iszabi CreditAttribution: iszabi commentedHi,
I 've got an error when I'd like to translate a field on contact form.
Error after saved translation filed:
Notice: Undefined index: field.storage.contact_message.field_mennyiseg in Drupal\config_translation\Form\ConfigTranslationFormBase->submitForm() (line 220 of core/modules/config_translation/src/Form/ConfigTranslationFormBase.php).
Drupal\config_translation\Form\ConfigTranslationFormBase->submitForm(Array, Object) (Line: 36)
Drupal\config_translation\Form\ConfigTranslationAddForm->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 583)
Drupal\Core\Form\FormBuilder->processForm('config_translation_add_form', Array, Object) (Line: 314)
Drupal\Core\Form\FormBuilder->buildForm('config_translation_add_form', Object) (Line: 74)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 574)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
call_user_func_array(Object, Array) (Line: 139)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 62)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 98)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 77)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 50)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 628)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
------------------------------------------------------
Latest patch (https://www.drupal.org/files/issues/2595535-55.patch) does not work for Drupal version 8.1.1. (released on 4 May)
Patch displayed errors:
Checking patch core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php...
error: while searching for:
namespace Drupal\config_translation\Access;
use Drupal\Core\Access\AccessResult;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Session\AccountInterface;
error: patch failed: core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php:2
error: core/modules/config_translation/src/Access/ConfigTranslationFormAccess.php: patch does not apply
Checking patch core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php...
error: while searching for:
namespace Drupal\config_translation\Access;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\config_translation\ConfigMapperManagerInterface;
use Drupal\Core\Access\AccessResult;
error: patch failed: core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php:2
error: core/modules/config_translation/src/Access/ConfigTranslationOverviewAccess.php: patch does not apply
Checking patch core/modules/config_translation/src/ConfigMapperInterface.php...
error: while searching for:
public function getLangcode();
/**
* Sets the original language code.
*
* @param string $langcode
error: patch failed: core/modules/config_translation/src/ConfigMapperInterface.php:203
error: core/modules/config_translation/src/ConfigMapperInterface.php: patch does not apply
Checking patch core/modules/config_translation/src/ConfigNamesMapper.php...
error: while searching for:
namespace Drupal\config_translation;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Config\TypedConfigManagerInterface;
use Drupal\Core\Language\LanguageInterface;
error: patch failed: core/modules/config_translation/src/ConfigNamesMapper.php:2
error: core/modules/config_translation/src/ConfigNamesMapper.php: patch does not apply
Checking patch core/modules/config_translation/src/Controller/ConfigTranslationController.php...
error: while searching for:
namespace Drupal\config_translation\Controller;
use Drupal\config_translation\ConfigMapperManagerInterface;
use Drupal\Core\Access\AccessManagerInterface;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Language\Language;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\Core\PathProcessor\InboundPathProcessorInterface;
use Drupal\Core\Routing\RouteMatch;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Session\AccountInterface;
error: patch failed: core/modules/config_translation/src/Controller/ConfigTranslationController.php:3
error: core/modules/config_translation/src/Controller/ConfigTranslationController.php: patch does not apply
Checking patch core/modules/config_translation/src/Exception/ConfigMapperLanguageException.php...
error: core/modules/config_translation/src/Exception/ConfigMapperLanguageException.php: already exists in working directory
Checking patch core/modules/config_translation/src/Form/ConfigTranslationFormBase.php...
error: while searching for:
use Drupal\config_translation\ConfigMapperManagerInterface;
use Drupal\Core\Config\TypedConfigManagerInterface;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\TypedData\TypedDataInterface;
use Drupal\Core\Form\BaseFormIdInterface;
error: patch failed: core/modules/config_translation/src/Form/ConfigTranslationFormBase.php:4
error: core/modules/config_translation/src/Form/ConfigTranslationFormBase.php: patch does not apply
Checking patch core/modules/node/src/Tests/NodeTypeTranslationTest.php...
error: while searching for:
* @var array
*/
public static $modules = array(
'config_translation',
'node',
);
error: patch failed: core/modules/node/src/Tests/NodeTypeTranslationTest.php:23
error: core/modules/node/src/Tests/NodeTypeTranslationTest.php: patch does not apply
------------------
Server environment:
mySQL: 5.6.19-67.0, for debian-linux-gnu (x86_64)
PHP: 7.0.7
Webserver: Apache/2.4.20
Note: After I've saved translated field, the results: "Successfully updated Magyar translation."
So after 'drush cr' the result has been displeyd on form successfull.
Comment #58
tstoecklerMaybe that's a chance for someone to work on a critical issue.
Marking Novice for the rerolling, the issue in general is of course not novice.
Comment #59
tstoecklerComment #60
esolitosI just tested the patch from #55: it applies successfully on
8.1.1
, on latest commit on branch8.1.x
and also on8.2.x
.Comment #61
tstoecklerThanks @esolitos! Then it just needs review now.
Comment #62
alexpottWhy are we storing the mapper as a protected property - I'm not sure it should be.
I don;t think this is needed anymore.
Comment #63
vijaycs85#62.1 - ConfigTranslationFormAccess is using the mapper updated from ConfigTranslationOverviewAccess in access method.
#62.2 - Fixed
Update: Applies to 8.2.x too.
Comment #64
Kristen PolI have tested the patch in #63 with the instructions from the summary:
and did not get an exception. Here is the a screenshot after adding the image field.
Based on the comments in #62 and interdiff in #63, this looks RTBC so marking it.
Comment #65
alexpottI'm not sure that this is a great way to pass around the mapper object. I've not got any ideas yet of a better way.
Comment #66
catchCan we just explicitly say we're calling the method to give it a chance to throw the exception?
Does this have to be !empty($target_language)? Can't just do $target_language since we always set it above?
Any reason not to use strict comparison?
Comment #67
nevergone CreditAttribution: nevergone commented#63 tested and works well!
Comment #68
catchCNR for #64/#65.
Comment #69
Kristen PolClarification: Needs review for #65 and #66.
Comment #70
catchYes! Off by one error...
Comment #71
manuel.afonso CreditAttribution: manuel.afonso commentedLooking forward for a solution, thank you all for your contributions.
If you have a partner using an interface language other than yours, you won't be able to edit content types any longer because of this issue.
I'm just starting with Drupal and couldn't apply the patch so I went for this temporary solution:
in ConfigNamesMapper.php
Comment #72
tstoecklerI think I came up with a way to solve #65 in a somewhat elegant way, would love to hear what you think. Also fixes #66. Unfortunately this makes the interdiff slightly unreadable, it might make sense to look at the actual patch instead, at least for the two access check classes.
Comment #74
tstoecklerOops, of course the method parameter name has to match the routing parameter. So not changing that, as changing the route path would be a little scope-creep-y in my opinion...
Comment #75
Gábor HojtsyDiscussed this with @tstoeckler on the multilingual meeting. The new access check inheritance looks nice to me, it does indeed solve the problem that @alexpott raised in #65. It also addresses all the problems raised by catch in #66, even though I did not find all the solutions at first, but @tstoeckler walked me through them. Looks good to go then :)
Comment #76
catchCommitted/pushed to 8.2.x, thanks!
Leaving at RTBC for 8.1.x, want to get a new release out for #2761865: Update from 8.1.3 to 8.1.4 breaks site seemingly permanently and also want to give this time before it gets into a tagged release.
Comment #79
catch8.2.x broke. Reverted and sending for re-test.
Comment #80
catchApparently messed up fixing the drupalcs issues locally, so uploading a patch for that.
Comment #83
catchOK that passed. Committed/pushed to 8.2.x and cherry-picked to 8.1.x. Thanks!
Comment #84
Benia CreditAttribution: Benia commentedShould I run #80 in an all-core site which is Drupal 8.1.6 or should I downgrade before that?
I wish to run it at best.
Comment #85
catch@Benia you can apply the patch to your 8.1.6 install (see https://www.drupal.org/patch) if you need the fix immediately, or wait about four weeks for 8.1.7.
Comment #86
penyaskitoGlad to see this one fixed. Thanks so much for working on it!
Don't we consider changing constructor an API break?
Comment #87
Benia CreditAttribution: Benia commentedIts the first time I run a core patch. When I did "git applay patch 2595535-80.patch" it seem to have failed but when I did "patch < 2595535-80.patch" It seems to have succeeded.
Please see attached image with errors from Ubuntu bash.
Comment #88
Benia CreditAttribution: Benia commentedComment #89
Benia CreditAttribution: Benia commentedComment #90
catch@penyaskito no explicitly don't consider them an API: https://www.drupal.org/core/d8-bc-policy#constructors
Normally we'd avoid changing them in a patch release, however given this fixes a critical bug and the constructor is specifically for a controller which should never be used directly (not even subclassed) I think it's fine in 8.1.x
Comment #91
Benia CreditAttribution: Benia commentedIts the first time I patch the core and I think I had a problem while patching. Can you please take a look at the attached file?
Comment #92
Benia CreditAttribution: Benia commentedComment #93
catchPlease open a new support request for applying the core patch and link to it from here.
You could also use the development release of 8.1.x (but better to do this on a test site first, same as patching).
Comment #94
Benia CreditAttribution: Benia commentedDid that. Please see here.
Comment #96
Heihachi88 CreditAttribution: Heihachi88 commented#80 worked. Drupal 8.1.7. When they will merge this patch in to the core?
Comment #97
esolitosPatch is committed on both 8.1 and 8.2.
Drupal 8.1.7 was a security release, so you'll find it in 8.1.8.
Comment #98
Gábor HojtsyRemoving from sprint, thanks all again!
Comment #99
@baux CreditAttribution: @baux commentedI've got a fresh Drupal 8.2.1 installation and I'm experiencing this problem.
In my site I've installed Drupal in english, made some configurations, added some modules and then I've activated "Configuration Translation", "Interface Translation" and "Languages" ("Content Translation" is not active).
I' don't have any error about languages in my recent log message, but the following message is shown in the filed(s) translation:
This error appears just on new content types... Articles can be properly translated.
I've seen that the patch should be included in the new releases, but I've unsuccessfully tried to apply it... the system says that it's already applied.
I've just updated the site to Drupal 8.2.2 with no solution to this problem.
Language configuration:
Don't know what to do... please help...
Comment #100
Alex Bukach CreditAttribution: Alex Bukach commented@baux, what you probably can do is to export one of configurations (
field.field.node.videos.body
orfield.storage.node.body
), modify it to match the languages and import it again.