[MGNLUI-3221] All authors share the same authoring locale Created: 28/Oct/14  Updated: 06/Aug/15  Resolved: 23/Apr/15

Status: Closed
Project: Magnolia UI
Component/s: forms, framework
Affects Version/s: 5.2.3
Fix Version/s: 5.3.9

Type: Bug Priority: Critical
Reporter: Mikaël Geljić Assignee: Mikaël Geljić
Resolution: Fixed Votes: 2
Labels: 5.3.9, i18n, support
Remaining Estimate: 0d
Time Spent: 3d 3.5h
Original Estimate: 7h

Issue Links:
causality
caused by MGNLUI-2696 i18n: As an editor I switch the langu... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

When opening the page-editor, the authorLocale is set on the I18NAuthoringSupport, so that form dialogs open directly in the same locale.

However, the I18NAuthoringSupport is a singleton (per-instance) configured component, which means concurrent authors may override the authorLocale from one another, e.g.:

  • superuser edits /demo-project in English.
  • eric edits /demo-project/about and switches to German.
  • superuser opens a dialog; she gets it in German instead of English.

This was introduced in 5.2.3 with MGNLUI-2696.

Couple options:

  • Either I18NAuthoringSupport should remain stateless and such synchronization of locale between editor and dialog should be performed differently (privileged option).
  • Either I18NAuthoringSupport should be a session or even sub-app singleton.


 Comments   
Comment by Benoit Olbrechts [ 15/Jan/15 ]

+1 on this ticket!
The current behavior leads to mistakes when 2 editors are working on the same page but different languages!

Comment by Vivian Steller [ 12/Mar/15 ]

This is a serious issue strongly affecting the usability of Magnolia in Multi-Site/Multi-Language scenarios. Please consider fixing this soon, seriously. Thanks!

Comment by Mikaël Geljić [ 01/Apr/15 ]

Hi Sang,
I see you've taken the second option, but have you tried the first one (privileged) as well?
In particular, we have to be careful if we make I18nAuthoringSupport session-scoped, because it's an observed "configured component". Thus it might register a certain number of observation listeners, one upon each session — as opposed to one for the whole system currently.
Cheers,

Comment by Sang Ngo Huu [ 01/Apr/15 ]

Hi Mika,

I've already tried first option, but it is quite hard to change the implementation of DefaultI18NAuthoringSupport to stateless. It set back the language to the object, need to have an object to keep the language for current user. It is fine to have an object per session. BTW, I will think carefully about your suggestion, I will try to use other object to keep language for dialog such as an object belong to session.

    @Override
    public void languageSelected(Locale locale) {
        if (locale != null && !locale.equals(currentLocale)) {
            this.currentLocale = locale;
            if (i18NAuthoringSupport instanceof DefaultI18NAuthoringSupport) {
                ((DefaultI18NAuthoringSupport) i18NAuthoringSupport).setAuthorLocale(locale);
            }
            doGoToLocation(getCurrentLocation());
        }
    }

Thanks,

Comment by Mikaël Geljić [ 01/Apr/15 ]

By stateless, I meant essentially removing the authorLocale from there — like it was before. The rest is fine, i.e. it's not sensitive to user sessions.

Storing the authorLocale sounds like something an existing component (FormBuilder maybe) or new component could do; as for the scope, we rather use "containers" (those ids in module descriptors) than the scope attribute, i.e. that would be a singleton in the admincentral container for example.

Comment by Sang Ngo Huu [ 06/Apr/15 ]

Hi Mika,

I have 2 solutions for this case, please help me give the advice
1. I commited to code to MGNLUI-3221_sn branch, you can see in the code, I set the locale value into session to use for each session by user. This case changes litle bit code, and more safer.
2. The rendered page will have an url in iframe:

<iframe class="gwt-Frame" width="100%" height="100%" allowtransparency="true" frameborder="0" src="/magnolia-enterprise-webapp/demo-project/de/about.html?mgnlPreview=false&amp;mgnlChannel=desktop&amp;currentLocale=en"></iframe>

So, I will add more parameter "currentLocale" to the link. Then in renderer, I will get the paramerer and add more attribute into comment (locale="en")

<!-- cms:component content="website:/demo-project/content/0" dialog="standard-templating-kit:components/teasers/stkTeaser" label="Internal Page Teaser" description="paragraphs.teasers.stkTeaser.description" locale="en" -->

Then in FormBuilder, I will get more parameter "locale" for building a form. And from class info.magnolia.pages.app.action.EditElementAction, I will read the locale value and put it to FormBuilder.

    public void buildForm(FormView view, FormDefinition formDefinition, Item item, FormItem parent, Locale currentLocale) {
        final String description = formDefinition.getDescription();
        final String label = formDefinition.getLabel();

This solution change a lot of code.

Thanks,

Comment by Mikaël Geljić [ 21/Apr/15 ]

Hi Sang,
Finally reviewing your proposals.
As an early impression, I think proposal #2 is not worth the pain, *might* be redundant too: we already propagate locale to page-editor.
See PagesEditorSubApp.setPageEditorParameters(DetailLocation) and DefaultI18NAuthoringSupport.createI18NURI(Node, Locale)
That said, it was still a good exercise!
Looking into proposal #1 now.

Comment by Mikaël Geljić [ 21/Apr/15 ]

So here's your second shot

  • As I said earlier, I18NAuthoringSupport is an instance singleton, so we want to get rid of authorLocale in there. In practice, we're *only * going to deprecate it because it's a public API.
  • Lately we tend to avoid introducing new dependencies to MgnlContext, because it's always referred to in static ways, hence is hiding a real dependency and is harder to test. We should use IoC wherever possible instead.
  • So what your commit did seemed like a "rather sad" indirection: going to a system singleton to ask for a user-dependent attribute. Nevertheless I do understand the driver behind that proposal—it did not require any change for client code at all.

Because we're mostly our own client there, we will go slightly further
In my last comment, I was mentioning "an existing component" should hold that locale.

  1. Scope.
    People may open multiple page-editor subapps. We will *not* try to share the authoring locale between them. Those pages might belong to *sites* that do not even have a compatible set of locales.
    => So we're going to keep the locale on a subapp component (talking about IoC container there).
  2. Where. At first I something like FormBuilder, but that one is not even a singleton in any container afaik.
    So maybe FormDialogPresenterFactory? could work but that sounds ugly.
    How about the SubAppContext itself?!

So I started a very little patch there to give you the direction, glad if you feel like pushing it further
https://gist.github.com/mkgl/f9569b2dc53c8006d677

Then we'll likely present that change to architects as well.
Thanks for your help!

Comment by Mikaël Geljić [ 23/Apr/15 ]

Got now a couple commits on branch MGNLUI-3221-mge.

First commit would be sufficient, second commit is "optional" because it's not something we currently do, i.e. sync page-editor locale when it's changed in dialog. We've always done it only the other way around, sync dialog locale when it's changed in page-editor.

We may as well preserve binary compatibility and avoid the API change in SubAppContext for 5.3.x, by casting to impl.
Changes will then be ported the new pages app—will eventually create a ticket there.

Comment by Mikaël Geljić [ 28/May/15 ]

Now integrated both changes:

  • Authoring locale is isolated per sub-app, i.e. page-editor
  • NEW: Sync back page-editor locale when it's changed in dialog
Generated at Mon Feb 12 09:04:25 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.