Problem/Motivation
Content translation inherits admin page status for translation pages from the edit page of entities. If the NodeAdminRouteSubscriber or AdminRouteSubscriber run after the content translation route subscriber though, this inheritance will not work. The ContentTranslationRouteSubscriber should run as late as possible.
Proposed resolution
Set priorities on ContentTranslationRouteSubscriber, NodeAdminRouteSubscriber and AdminRouteSubscriber relative to each other, so content translation goes last.
Remaining tasks
Tests, review.
User interface changes
Admin route inheritance will work proper.
API changes
None.
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedFor example, try custom menu links.
Comment #2
YesCT CreditAttribution: YesCT commentedthis was fixed in part 4 #2301317: MenuLinkNG part4: Conversion
with
Comment #3
YesCT CreditAttribution: YesCT commentedthat might have been adding the wrong relate issue.
maybe meant to add #2276387: Translate routes should properly inherit admin path use from edit route
Comment #4
dawehnerThis was actually the wrong fix as it fixed the symptoms, but not the reason.
Comment #5
YesCT CreditAttribution: YesCT commentedok. when we fix it the correct way, we should check to see if the similar fix for nodes in 2276387 can be undone also.
Comment #6
YesCT CreditAttribution: YesCT commentedComment #7
Gábor HojtsyShortcuts have the same problem, see #2320037: Non-fieldable entities (with only base fields) cannot be configured translatable, eg. shortcuts which includes the same workaround that is in menu_link_content.
Comment #8
Gábor HojtsyDoes any of the existing ones have tests, besides node translation pages?
Comment #9
Gábor HojtsyComment #10
Gábor HojtsyDuh, not going to work. Instead of service priorities, we should set event priorities. This works.
Comment #11
Gábor HojtsyNow with tests. The interdiff is the TEST ONLY patch. It should not fail since we have the hardcoded admin page designation in the route. Once we remove that, but without fixing the priorities, it will not work (see TEST ONLY WILL FAIL patch). The fix and test overall is in the 11 patch.
Comment #13
penyaskitoMakes sense to me.
Comment #14
alexpottHow about
-210
so there is a gap between AdminRouteSubscriber and ContentTranslationRouteSubscriber?Comment #15
Gábor HojtsySure thing. The only side effect is that requires the param converter route subscriber to be changed as well since otherwise they are the same priority and the params are not yet converted when access tries to deal with them (content translation comes before paramconverter in the alphabet).
Comment #16
Gábor HojtsyBetter said, the content translation routes would not have param converters applied properly if the content translation routes are added after the param converter altering is done. (This shows in severe test fails if we don't change the param converter priority).
Comment #17
Gábor HojtsyAdding blocker tag since this postpones #2320037: Non-fieldable entities (with only base fields) cannot be configured translatable, eg. shortcuts.
Comment #20
Gábor HojtsyWas random test fail.
Comment #21
alexpottCommitted 3254a1a and pushed to 8.0.x. Thanks!
Comment #23
Gábor HojtsyYay, thanks!
Comment #24
dawehnerGreat work!