Problem/Motivation
Links to content translations on translate tabs are incorrect as of #1987882: Convert content_translation routes to a new style controller. They always lead to the entity in the current language. Screenshot is for nodes, but same applies to menu items, shortcuts, etc. Screenshot from a site where all three languages had their path prefixes configured. The bug is not path prefix configuration dependent.
Proposed resolution
Fix. Write tests. Review. Commit.
Remaining tasks
Most of them.
User interface changes
Links will be correct.
API changes
Likely none.
Comments
Comment #1
Gábor HojtsyComment #2
Gábor HojtsyThis is the patch I originally thought would be good, but it only fixes symptoms on the overview screen. Not in views of course. Not sure how this all worked before?! At least the test modifications here can be used as-is, they will test all entity types that inherit from this test baseclass, eg. nodes, menu, shortcut, etc.
Comment #3
Gábor HojtsyRemoved the views use case because that was broken before and down to other things happening in views. Will look up which issue exactly.
Comment #4
Gábor HojtsyThe delete links are translation specific so the data loss argument does not stand :/
Comment #5
Gábor HojtsyComment #6
Gábor HojtsyThe views problem is covered in #2217569: Fields row plugin: Translation is non-uniform for base fields, Field UI fields, links; no way to choose "this row's language". Posted the views screenshot there.
Comment #7
andypostAlso there's related bugs:
#2323935: Translations page for content created prior to enabling shows "unpublished" for English versions
#2251907: Wrong status on Translate tab for entities - suppose this one could be closed as duplicate once "add" addressed in previous issue
Comment #8
Gábor Hojtsy@andypost: I don't think either of those are duplicates of this one.
Comment #9
penyaskitoIf I'm right I broke this at #1987882-195: Convert content_translation routes to a new style controller. test_coverage++
Comment #10
andypostTested the patch with simpletest.me and found that redirect after adding a translation is strange.
Once term is created in
en
and then I add translation forru
, after submit I redirected to edit foren
- that's confusing.Suppose that's not a regression but it makes sense to fix here
Comment #11
Gábor Hojtsy@andypost: what does it have to do with the translation overview screen?!
Comment #12
andypost@Gabor link to add is generated on overview page, and I suppose to add "destination" for add link here
Comment #13
plachI think we should keep using
LanguageManagerInterface::getSwitchLinks()
otherwise sites using session language selection won't work.Comment #14
Gábor Hojtsy@plach: I was half-way back-refactoring this code... But then... Why do we need the links from switch links?! It should be enough to link to view/edit, etc. pages by specifying a path/route and a language and the outbound URL modifiers will do what they need, no? They should. Otherwise why don't we need to use the switch links at *every* place where we need to link to language specific versions? Why are they needed here as a special case? OutboundPathProcessorInterface is implemented by LanguageNegotiationSession, so the same patch should work for that use case, no? Well, in practice it looks like it does not work... While trying this on a local site. But that is quite shocking to me. How are we supposed to link to translated entities then? No way to just pass in a language to a URL generator, but always need to refer back to languagemanager's switch links? Really?
Comment #15
plachUnfortunately the only reliable way to obtain language-aware URLs is getting language switch links. This has been true since D7 and is precisely the reason why I always wanted to work on #1137074: Make obtaining language-aware URLs less painful, but I could never get to it. Probably soon will be the moment. Anyway, to fix this for now we need language switch links, hopefully we can get rid of the language manager method (or probably making it wrap the new code, to preserve BC) and get a more sane DX afterwards.
Comment #16
Gábor HojtsySigh, that means that fixes we thought were correct in #2149649: Entity rendering/theming does not use the active entity language to render links etc. are not general enough then... :/ The main problem is factoring this back to switch links will make it use paths all over again which the whole factoring away from paths to routes was supposed to avoid. Sounds insane to factor code from routes back to paths now... getLanguageSwitchLinks() on the language manager is path based. So to make that route based sounds like resolving #1137074: Make obtaining language-aware URLs less painful as part of this issue or postponing this one on that which means almost no way to translate entities for quite a long while (at the rate of these issues being fixed). And I hoped I can show the multilingual UI with a straight face in Amsterdam :/
Comment #17
plachOk, let's fix this for the 80% of use cases now and address the remaining 20% in #1137074: Make obtaining language-aware URLs less painful.
Comment #18
vijaycs85+1 to RTBC.
easy to get conflicted and hard to reroll.
Comment #19
webchickI guess this works for now. This seems extra cumbersome to have to remember to add that parameter manually (esp. since you already gave it a language 3 lines above that). It's not clear to me in what way #1137074: Make obtaining language-aware URLs less painful would help with the DX here.
However, the issue is borderline critical and the patch fixes it (with test coverage so we don't break it again) so...
Committed and pushed to 8.x. Thanks!
Comment #21
Gábor Hojtsy@webchick: no this is actually the simple way to do links, but it does not work for all cases of language negotiation setup, notably for session language negotiation. (I think it should btw, but it clearly does not).
Comment #22
Gábor HojtsyComment #24
tstoecklerThat the link to the translation itself got the language option is correct, but the the add, edit, and delete links this is not correct IMO. Translators should not be forced into a context switch unnecessarily. The actual problem is that the links point to the entity edit forms in the first place instead of the translation edit forms. See #2451693: Operations links on the translations overview page do not link to the correct route