Convert language globals to contexts

I'm on this one.

This is just gonna be so awesome!

* global $language is gone!
* one bootstrap phase is gone!
* language keys are now lazy-initialized!

I haven't been able to run all tests locally. But things doesn't seem to explode at least, though I get some strict warnings during manual install.

Anyway, this is a start.

Would it be possible to have the test bot running in this sandbox? It would of course help tremendously when doing these kinds of refactoring.

Can the test bot even run on sandboxes? And if it could, can it run on projects that happens to be core projects/forks?

OMG, YEAH!

Great work @dixon_, I get a notice and a strict warning on /install.php - this appears to be due to the fact that the "is Drupal installed?" check is done in _drupal_bootstrap_database() and butler/context is not instantiated until the next bootstrap phase, namely _drupal_bootstrap_variables() - not sure how best to fix this?

Sub

Well... not at all sure if this is the way we see things going but I managed to fix the notice/warning by adding a DRUPAL_BOOTSTRAP_CONTEXT phase to fire up context early on in the bootstrap process.

Since we don't want to support stdClass objects long-term anyway, is this a good time to move to $context['language:ui:language'] (or whatever the internal structure there is)?

Our intention is actually to make the language object more formal. Not sure whether it's going to be a non-stdClass with true interface or abstract parent, but at least, I think we should consider it as some kind of an entity that's holding full language and locale information. For now, and for the sake of making progress independently with contexts, we should retain it as an object.

sub.

@Gabor:

$context['language:ui']

I thought we agreed on $language_interface: #1222194: Rename global $language to $language_interface.

@plach: I did not want to hold this back for that discussion. We can search and replace even in the patch to have ui to interface, so does not seem like a fundamental thing that would change the direction of this patch in any way. I do agree language:interface could be better for understandability, should be a simple change in the patch.

BTW marking #1222194: Rename global $language to $language_interface as obsoleted by this.

Oh, and since this issue renames the main language key (from language to language:ui or language:interface or whatever), the related configuration options do change as well, so they need to be updated. See the update related portions of #1222194: Rename global $language to $language_interface which now need to be incorporated in this patch. Marked that one as duplicate.

Fixing title back.

Tagging for Multilingual Initiative.

Subscribing...

@Crell: Yeah, I agree language:ui is easier to type, however, we already had a long discussion in #1222194: Rename global $language to $language_interface to make it "interface" not "ui". @sun and @plach were among proponents. It is easier to understand I guess. (My initial gut was language_ui as well there).

@Crell: On the stdClass question, at least in D8MI, we are trying to move in smaller digestible chunks and we have a lots of adjacent APIs that need cleanups and have them on the way. We lack real object save (#1215716: Introduce locale_language_save()), delete (#1260528: Introduce API to delete languages), load (#1260510: Introduce a language_load($langcode)), etc. APIs. Many places use direct queries to delete, update, load languages. So we need to abstract that out anyway, and apply that to the code that uses it (#1260520: Apply language_save and language_load to the locale management UI among others). All this is just to say that we are very well aware of our shortcomings in that area, and we still need to get abstractions of the real API components worked out and landed. Then we can put that under an OO interface to allow operations on a language object directly, or whatnot. It would be great if context would support the stdobj for now, and we can move forward later looking at where other parts of Drupal move. There is just too much to catch up with in the language subsystem that I'd love to follow example (in terms of general Drupal patterns) instead of being derailed to set example there :)

@dixon_: looked at your code and cannot understand why did you use this $types at all. It seems to interfer with the use of $types later in the method in ContextHandlerLanguage's getValue():

+    if (empty($types)) {
+      $types = language_types();
+    }

Also, I did not look at how this maps into the larger picture of context, but would like to ensure that the context initializer will not run before it used to be in the bootstrap. We are supposed to have a user context set up for example by the point we need to work with languages, because the page language might depend on user session or user setting depending on configuration. So there are certain pre-requisites that we so far satisfied with the bootstrap ordering. With that lost, how can that be ensured?

Also, I did not look at how this maps into the larger picture of context, but would like to ensure that the context initializer will not run before it used to be in the bootstrap. We are supposed to have a user context set up for example by the point we need to work with languages, because the page language might depend on user session or user setting depending on configuration. So there are certain pre-requisites that we so far satisfied with the bootstrap ordering. With that lost, how can that be ensured?

I had a look at this, currently context is initialised in the variables bootstrap phase, this is too late for install for example where the only bootstrap phase initially loaded is configuration - this causes the notices and warnings @dixon_ was experiencing.

I experimented locally with a context bootstrap phase after page_cache which seemed to address the problem but I haven't had the time yet to fully explore that avenue...

@Gabor About $types -- I'm not sure either why I did so :P I just had a big copy-paste mania and somehow got it working. Hey, one need to start somewhere ;)

@Gabor About the bootstrap -- In the bootstrap, or in the system implementation of hook_context_info we are just registering the language handler. All the handlers gets registered there. And the nice thing about it is, if you'd like to use $context['user:language'] or whatever, that's lazy-loaded for you! And if you don't need it, it's not loaded. #win

Hopefully I will find time to look at this again soon.

Yeah, @matason is right. Initializing earlier is needed for the install system etc. But it's not necessarily needed to be able to grab other pieces of context. After all context handlers are registerd (which happens at the same time), they will all be lazy loaded.

@dixon_: yes, that assumes that users are used from the context as well (in language initialization), which I guess will be / is ensured by another patch, so that should be safe :)

working on this

Here is an apparently working patch, I tested it for a while and it seems to be working really well, even the upgrade path :)

I tried to address the concerns/feedbacks above, with an exception: ContextHandlerLanguage is still in context.inc since it cannot live in language.inc which is conditionally loaded only on multilingual sites (see ContextHandlerLanguage::getValue).

To solve the installer problem, I introduced as suggested a context bootstrap phase just after the configuration bootstrap: since we have no DB support there we cannot invoke hook_context_init() implementations. The only working solution I could come up with is instantiating a child context and allowing for registeration of hard coded low level context handlers. This child context is only used from code requiring context before variable bootstrap, otherwise it is overridden by a fully initialized context where hook_context_init() implementations have run regularly.

@Crell:

Do you mean this one should somehow depend on the user patch? Actually the bootstrap refactoring was needed only to avoid a couple of notices in the first install screen (and probably getting it localized :). So I can split off that part and go on without it to speed up things. Sounds good?

Edit: I had a look to the user patch, it does not addresses the problem we have here: bootstrap_invoke() cannot be called at all without database bootstrapped, due to the cache_get() calls in it.

Rerolled the patch to fix the couple of minor things below.

+++ b/modules/locale/locale.install
@@ -255,3 +255,67 @@ function locale_schema() {
+/**
+ * Returns the new names of core language types. ¶

Trailing whitespace.

+++ b/modules/locale/locale.module
@@ -218,12 +218,20 @@ function locale_menu() {
+ * Implements hook_context_init().
+ */
+function locale_context_init(DrupalContextInterface $context) {
+
+}
+
+/**

Unneeded hunk.

Because of new namespace why not call objects LanguageHandler instead of ContextHandlerLanguage seems more natural? Symfony does this too, if you are worried about conventions.

EDIT: Maybe this comment should move to another issue (maybe a naming convention core issue again?)

Crell: any feedback? Would love to see this in if it passes your review :)

@pounard:

Not really up-to-speed with namespaces and naming conventions, but I guess that context handler names will be somehow related to the name of the file that will host them, which is yet to be decided. So perhaps we should postpone this decision.

@Gábor Hojtsy I see that in the handler implementation you propose, you have two languages, language:ui and language:content, now that it's being contextual, could this be redundant with the fact that context can be overriden?

I mean, I had an idea for "tooling" (like contextual links): under each fragment or page we render that is actually "tooling" we could override the context, and set the language to be the one the user configured for itsef (or default site language), this would create this kind of behavior:

1. Site default language: "en"
2. Content page hit, context is overriden, language become the content one ("es" for example)
3. Contextual link in content, context overriden once again: language become the user one ("hu" for example)

And then you have a totally weird web page with three languages in it! :)

In order to achieve this kind of algorithm we don't need to differenciate ui and content, but you might have a totally different idea in mind.

@platch Yes my question wasn't asked in the right place though.

@pounard: UI and content language are two entirely different languages with different concepts, not interchangeable. Actually, there's even a third, URL language.

+++ b/includes/bootstrap.incundefined
@@ -112,29 +112,29 @@ define('DRUPAL_BOOTSTRAP_CONFIGURATION', 0);
- * Third bootstrap phase: initialize database layer.
+ * Third bootstrap phase: initialize context.
  */
-define('DRUPAL_BOOTSTRAP_DATABASE', 2);
+define('DRUPAL_BOOTSTRAP_CONTEXT', 2);
 
 /**
- * Fourth bootstrap phase: initialize the variable system.
+ * Fourth bootstrap phase: initialize database layer.
  */
-define('DRUPAL_BOOTSTRAP_VARIABLES', 3);
+define('DRUPAL_BOOTSTRAP_DATABASE', 3);
 
 /**
- * Fifth bootstrap phase: initialize session handling.
+ * Fifth bootstrap phase: initialize the variable system.
  */
-define('DRUPAL_BOOTSTRAP_SESSION', 4);
+define('DRUPAL_BOOTSTRAP_VARIABLES', 4);

This change should be separated - The changed numbers of bootstrap is serious change to be inside language patch

Talked about this with @Crell on IRC. Some notes:

1. @Crell is about to write up docs on how to do context handlers. Some things changed since this patch was rolled and need to be updated.
2. Re @andypost, we can skip renumbering the bootstrap items, but we'll remove one and put a hole in there for now to limit the scope of this patch.
3. I've asked him to provide more feedback once/if possible.

I spoke with @Crell in IRC too: he confirmed that the current code needs to be updated to reflect the latest WSCCI changes.

Wrt the bootstrap issue, it seems it will be solved when CMI will land:

Bootstrap would look like: 1) Init config system. 2) Init context using config. 3) Map to responder object using the context. 4) Print $responder->dostuff().

Hence we agreed that, since the bootstrap sequence above should cover every "early-bootstrap" scenario on installed drupal, we just need to special case the installer script:

plach- Crell: just saying, if your plan covers any "installed drupal" use case, than special casing the installer would be perfectly fine for me
Crell- I think the installer should just have some hard coded setup logic that associates the http handler and language handler.
plach- Crell: works for me
Crell- Yeah, I *think* we've got "installed drupal" covered;.

We agreed on the following roadmap:

1. reroll the patch to only introduce the language handler + enough unit tests to verify everything
2. merge it in
3. roll another patch actually migrating everything and special case the installer

Additionally we talked about generalizing the language negotiation fallback logic through a ChainedContextHandler, which would allow to reuse it wherever needed (see the attached log for details).

Sounds like a great plan. @plach: can you continue work on this? Thanks!

Sure :)

#1269832: Use Symfony / context code to retrieve HTTP language preferences is related.

Per catch, and blessed by Larry, moving this and all other WSCCI issues to the Drupal core queue.

Should we keep rollling it against the sandbox or mark postponed for core until basic context infrastructure land?

Tagging for base language system.

Good waking up this issue, but still postponed, I have a question thought:
@sun #36 You never explained why UI and content language are totally different contextes. The whole goal is to display stuff in the user language, with the maximum accuracy. Whatever is the paradigm to achieve this -and my idea is one of them- UI remains different, it's just the implementation mean and discover mecanism that differs. Content can carry a default language and translations, the goal is still to display the one that is the best for the user, and once again, my idea does not cut that out either. Could you be more precise?

@pounard: the difference between interface and content language is already in Drupal. This issue was/is all about making the interface language more explicitly named (which the introduction of contexts was/is also about to do anyway, so it is happening anyway if we don't want to do it as a global).

Imagine this situation. You want an English interface, let's say a document repository like google.com. You use the interface in English but content in all kinds of languages are shown. It is not filtered for your interface language and even if you want it filtered, its a different control from your interface language setting, which is under your preferences (vs. content language which is something applied to the page content displayed).

Yes, the idea I throwed supports that nicely.

@pounard: Sounds like you're not familiar with the language negotiation system in Drupal core. The system was intentionally designed to be very flexible to account for custom use-cases, as that has been a total show-blocker for certain sites in the past. FWIW, even the language negotiation UI provided by Drupal core leaves out some details and auto-configures a range of hard-coded assumptions under the hood in order to simplify it for the 80% use-case.

Hum, I didn't say the opposite, it's not revelant to me. I justed wanted clarifications but now I have them, nothing I didn't understood bafore. Thanks for the time thought.

Tagging for language negotiation too.

I suppose this is a won't fix given #1233232: Add unified context system to core is a won't fix and this would be dependent on that.