While working on #1471432: Rework language_list(), let people use more special languages I stumbled into our broken support for disabled languages. You can assign disabled languages to nodes but only if you have translation module enabled. Still then you cannot assign disabled languages to path aliases (which nodes might create) or stage interface translations a similar way. This seems to be the only feature of disabled languages, and it otherwise provides for lots of complication to try and be supported on the backend but not on the frontend. It certainly complicated #1471432: Rework language_list(), let people use more special languages to a point where it did not make sense to support anymore. Chatlog with @catch about the motivation:
[07:44am] GaborHojtsy: catch: hi
[07:45am] catch: Hi GaborHojtsy
[07:47am] GaborHojtsy: catch: I wanted to tap your mind for an interesting question "disabled languages" and what does that mean - in D7, if you disable a language, you can still assign it to content for example, but it would not show in the language switcher block; you cannot assign disabled languages to path aliases though, so when the content gets a path alias, you cannot edit it on the path alias admin, etc.... I stumbled into this problem when unifying the language lists available in http://drupal.org/node/1471432 (and all test fails are related to the previous assumptions)
[07:47am] Druplicon: http://drupal.org/node/1471432 => Rework language_list(), let people use more special languages => Drupal core, language system, normal, needs work, 49 comments, 10 IRC mentions
[07:47am] GaborHojtsy: catch: so the patch currently tries to walk that fine line "better" of allowing assignment of disabled languages to stuff but then not allowing the use of those things on the fronted... so you could assign a path alias to a disabled language but not use it, etc.
[07:47am] catch: GaborHojtsy: is this punishment for http://drupal.org/node/356036 ?
[07:47am] Druplicon: http://drupal.org/node/356036 => Allow creating nodes in disabled languages => Drupal core, translation.module, major, closed (fixed), 61 comments, 4 IRC mentions
[07:48am] GaborHojtsy: catch: yeah
[07:48am] GaborHojtsy: catch: so I'd be much happier if we would remove disabled languages to appear anywhere
[07:48am] GaborHojtsy: catch: people basically use this as content staging in new languages
[07:48am] catch: GaborHojtsy: it looks like that's what I originally wanted
[07:48am] GaborHojtsy: catch: before they introduce new languages
[07:49am] catch: GaborHojtsy: so I would not be at all opposed to making disabled really mean disabled.
[07:50am] catch: GaborHojtsy: and if people want to do content staging they can override either end of this to allow that workflow?
[07:51am] GaborHojtsy: catch: yeah, I think those people should use per node permissions or something to hide nodes in staged languages
[07:51am] GaborHojtsy: catch: this quickly spirals into other things, like menus in disabled languages, and you know....
[07:51am] catch: GaborHojtsy: yep.
[07:51am] GaborHojtsy: catch: so I'd better for "disabled" to just mean "you cannot see it on the site anymore but not yet deleted"
[07:53am] GaborHojtsy: catch: disabled languages are an interesting concept either way... what do we do with data (node, path alias, etc) that has a disabled language assigned (from before it was still enabled)
[07:53am] catch: GaborHojtsy: you've seen the disabled modules issue?
[07:53am] GaborHojtsy: catch: no
[07:53am] catch: GaborHojtsy: http://drupal.org/node/1199946
[07:53am] Druplicon: http://drupal.org/node/1199946 => Ensure that disabled modules maintain data integrity => Drupal core, base system, major, needs work, 87 comments, 23 IRC mentions
[07:54am] catch: GaborHojtsy: that contains my general feeling on 'disabled', applies to languages as much as modules.
[07:55am] GaborHojtsy: catch: so I'm constantly thinking what should disabled language mean; if I see a node in the node admin overview in that language, should I see the language name? should it say (disabled)? Should users be able to access content (eg. nodes) in a disabled language or does that hide all content in that language? or would it just remove all navigation to that language from the public? what if its a custom menu item? eh.
[07:55am] catch: GaborHojtsy: I think it's the sort of feature we should seriously just look at dropping because it's a big headache for very little gain.
[07:56am] catch: It's content depending on configuration, then removing the configuration from under the content's feet.
[07:56am] GaborHojtsy: catch: yeah
[08:00am] catch: GaborHojtsy: does that help at all?
[08:00am] GaborHojtsy: catch: yeah, I read up on the modules issue
[08:01am] GaborHojtsy: catch: similar case, I was really walking on eggs in http://drupal.org/node/1471432 to try and figure out when to allow disabled languages and when not
[08:01am] Druplicon: http://drupal.org/node/1471432 => Rework language_list(), let people use more special languages => Drupal core, language system, normal, needs work, 49 comments, 11 IRC mentions
[08:01am] catch: GaborHojtsy: I think I've lost that one, but I'm just going to never ever fix any issues to do with disabled modules ever again
[08:02am] GaborHojtsy: catch: I think I should mark the language issue postponed and open one for removing support for assigning disabled languages at least
Attached a non-tested quick patch based on my work from #1471432: Rework language_list(), let people use more special languages.
Comment | File | Size | Author |
---|---|---|---|
#27 | drupal8.path-test.27.patch | 510 bytes | sun |
#23 | drop-disabled-languages-23.patch | 41.43 KB | Gábor Hojtsy |
#21 | drop-disabled-languages-21.patch | 41.43 KB | Gábor Hojtsy |
#19 | drop-disabled-languages-19.patch | 39.12 KB | Gábor Hojtsy |
#15 | drop-disabled-languages-15.patch | 39.07 KB | Gábor Hojtsy |
Comments
Comment #2
andypostLet's take another issue into account #400450: Be more graceful when editing nodes with disabled node types
Suppose we should work with all disabled controls by the same way.
For example when some filter format is used but not available we have fall-back format, probably we could have a fall-back to system language or node-defaults
Comment #3
Gábor HojtsyNo, please no, no, no :( Supporting disabled languages is a nightmare as you think about it trickling down different areas. See the chatlog above. If you support it on nodes only on translation, that is an impossible feature. You need to support it on nodes without translation then (eg. in overviews, when you create new content only in that language "for staging" as the comment in the removed code stated the intention for this mis-feature). We need to support disabled languages on aliases, so the node alias can be saved. Then we need to be able to navigate to this page, which is impossible if the language is disabled and you use field translation, so we'd need to support getting to pages in disabled languages (configuring and supporting path prefixes for languages in disabled languages too). Same for blocks, and so on and on. So what is the difference of disabled or enabled languages then, since everything is so interconnected that it trickles down to supporting it at all places.
Comment #4
andypost@Gábor Hojtsy I scare that dropping this support we loose ability to enable language version of site with one click. I see only one purpose of disabled language is to provide ability to make some language version of site without publishing.
How do we change workflow for creating multilingual sites then?
Comment #5
Gábor Hojtsy@andypost: the point is you already cannot do that. It does not work for field translation because you cannot configure path prefixes and domains for a disabled language, and it will not show disabled languages in switchers, so you cannot switch to it otherwise. It does not let you assign path aliases in that language, it does not unpublish the menu items you create for the nodes, etc. It does not even work for nodes and it is not going to be supported for other entity types... And the list goes on and on. If you need content staging, use solutions for that. Node/entity access, staging servers, etc. It is just as likely that you need to stage a new language or a new menu section or a new subsite. These involve menu settings and pieces of content. Disabled languages nevet spanned that, it only allowed for node assignment just as an accident that grew into a kind-of-feature. It provides so broken a benefit and is so complex to fix (basically solve the content staging problem just for languages), that it is pointless.
Comment #6
Gábor HojtsyChanged the tests too that worked with disabling / enabling languages. There are lots of opportunities to clean up tests, I was surprised that the path.test had tests for testing path prefixes in foreign languages (has nothing to do with functionality in path module at all). Trying to focus this issue on just removing the deadly broken support for disabled languages though.
Comment #8
plachFWIW, altough being one of the persons that actually made disable languages assignable to nodes, I agree that using to proper tools for staging site modifications should be the right way. It looked like a valid feature way back then but the experience teach us that it wasn't that great idea after all.
Aside from staging content the idea of disabling languages was useful only to remove native english from the accessible languages. In D8 this is not need anymore and if we don't support the ability to stage new languages, since there are better ways to do that, I'd say it makes totally sense to drop support for disabled languages altogether. This will make the UX simpler besides the already stated DX advantages.
Comment #9
andypostWhat's about removing "enabled" and introducing "published" ?
probably we should close #1314250: Allow filtering/configuration of which languages apply to what (UI, nodes, files, etc)
Comment #10
Gábor HojtsyNo, #1314250: Allow filtering/configuration of which languages apply to what (UI, nodes, files, etc) still makes sense, eg. we need to let users remove all kinds of special languages from node assignment if they want to as a usability feature. It might or might not be contrib though.
Comment #11
Gábor HojtsyFixed the remaining tests that assumptions about disabled languages. Added language deletion and addition in their place. As expected, this was very limited to certain obscure areas in the tests even. It feels so great to do this cleanup :)
Comment #12
andypostPatch is good except comments and some assert messages.
Terminology should be reviewed - should we use Installed or Added languages?
enabled? seem added is better
s/enabled/added/
same
this comment should be fixed too
Delete fr language
Maybe it's better idea to use Installed not Added language
Comment #13
Gábor HojtsyI think after this patch, enabled == installed == added == existing == present == setup == configured, etc. English is a flexible and versatile language, I don't think we need to unify this (right now at least).
So the language_list() comment that you noted still says enabled could be:
After this patch, all these mean the same thing :)
Comment #14
andypost+1 to * Returns a list of configured languages.
Language could be configured(installed) via Add or Import
Comment #15
Gábor HojtsyOk, picked "Returns a list of configured languages." for language_list(). Fixed the other places you noticed and a couple more where enabled languages were specifically called out.
Comment #16
kalman.hosszu CreditAttribution: kalman.hosszu commentedI agree with andypost's comments. I checked the code and everything is fixed, so I change the status to RTBC.
Kálmán
Comment #17
catchHave a feeling the user module decoupling patch conflicted with this, marking for re-test.
Comment #18
catch#15: drop-disabled-languages-15.patch queued for re-testing.
Comment #19
Gábor HojtsyOnly one hunk failed in locale.module related to the user module move-around indeed. Rerolled. Let's wait for it passing and then should be back to RTBC.
Comment #20
Gábor HojtsyLet's get this in then :)
Comment #21
Gábor HojtsyMade the update function inline in upgrade bootstrap as per @catch's recommendation in IRC (we are not supporting D8-D8 head updates still). Also moved the previous update function to there on his request. Its a minimal change. Should still pass.
Comment #23
Gábor HojtsyMissed one column rename when copying the count update code.
Comment #24
Gábor HojtsyOk, so this one just moves the update code from update functions that would never have run to the upgrade bootstrap as per catch's suggestion. Should be still good to go.
Comment #25
Dries CreditAttribution: Dries commentedReviewed; it is actually a nice code simplification in addition fixing a usability wtf. Committed to 8.x.
Comment #26
Gábor HojtsyAdded a change notice at http://drupal.org/node/1548406
Comment #27
sunComment #28
sunSorry, wrong alarm. You're not guilty.
#1497230: Use Dependency Injection to handle object definitions is.