Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug with nested fields & translation #962

Open
DigitalGoldfish opened this Issue Dec 25, 2018 · 15 comments

Comments

Projects
None yet
4 participants
@DigitalGoldfish
Copy link

DigitalGoldfish commented Dec 25, 2018

Posted the same issue here: https://discourse.getcockpit.com/t/html-wysiwyg-field-nested-in-set-does-not-update-content-when-switching-content-language/481

I am running into a bug with localized fields nested inside of sets:

Example collection item:

Text : type text
HTML : type html
SET : type set
NestedText : type text (nested in Set)
NestedHTML : type html (nested in Set)

All fields have the localize option enabled. I do have two languages: default + EN

When I now enter some values and then use the button to switch from the default language to EN the two text fields and the HTML field are emptied, however the NestedHTML field continues to hold the content. Modifying the NestedHTML fields content and then switching back to the default language will also not reset the fields contents to the default language.

I had a look at the code and I traced the problem back to this few lines in the field-html.tag (same for field-wysiwyg.tag)

if (editor && this._field != field) {
editor.setContent(this.value || '');
}
When the field is not nested in a set the value of the “field” variable will change from “HTML” to “HTML_en”. However when the field is nested inside a set the field name does not change as it is the name of the set that changes.

What purpose does this field name comparison have? Can I savely omit it to get translation working for me? I am under a bit of pressure and do have to fix this bug rather quickly …

@aheinze

This comment has been minimized.

Copy link
Member

aheinze commented Dec 31, 2018

I'll look into that! Thanks for reporting!

@DigitalGoldfish

This comment has been minimized.

Copy link
Author

DigitalGoldfish commented Jan 13, 2019

if (editor && this._field != field) {
editor.setContent(this.value || '');
}

The downside to removing this few lines of code is that the cursor will always reset to the first character in the editor field - that's not awesome and makes editing cumbersome but as a temporary workaround it is fine for me, especially as there is not going to be a lot of editing.

I also learned that Image fields have a similar problem - their values do not get changed/cleared when switching language.

@rootzjg

This comment has been minimized.

Copy link

rootzjg commented Feb 21, 2019

We have the same problem with image fields (I first thought it was a feature that it was showing an image from default language when switching to another language) but also with localized markdown fields.

For localized markdown fields I did some testing and seems like the first markdown field works correctly but if the collection has other markdown fields (we have three) content for the rest of them gets overwritten from the previously selected language when switching language in the editor. Even though they have localized content already saved. The correct content will show when you refresh the page.

So this bug is something a bit more general than just nested fields. Maybe worth a new issue or renaming this one?

@rootzjg

This comment has been minimized.

Copy link

rootzjg commented Feb 21, 2019

Investigated a bit more with those localized markdown fields.

Debugging javascript shows that when switching language evtSrc is false for the first markdown (field-html) field and the editor gets updated. Then for the following markdown fields evtSrc is true and that's why setValue isn't called and the editor is showing the content from the previously selected language.

            if (editor && !evtSrc) {
                editor.editor.setValue(value || '', true);                 
            }

Removing the evtSrc check causes the editor to reset to the first character in the editor field. As I'm not a js pro and not familiar with the code/framework I couldn't yet figure out a fix. Might be a bit easier for someone who is familiar with the code?

@aheinze

This comment has been minimized.

Copy link
Member

aheinze commented Feb 21, 2019

@rootzjg can you share your collection definition? (located in storage/collection)

@rootzjg

This comment has been minimized.

Copy link

rootzjg commented Feb 21, 2019

@aheinze thanks for looking into this!

See Games.collection.txt

The problem is with the localized images: banner, image1, image2 and with the markup fields description2, gameProperties (description1 works ok probably because it's the first one?).

Moving from one language to another seems to first work ok at first but when you start going back and forth between them the problems start.

We have four languages in addition to the default language.

@mpartipilo

This comment has been minimized.

Copy link

mpartipilo commented Feb 21, 2019

bump

Just found out I'm having a similar issue. I don't have nested fields, but multiple markdown fields.

@aheinze

This comment has been minimized.

Copy link
Member

aheinze commented Feb 21, 2019

I've committed some updates. Could you please test the latest next branch? @rootzjg @mpartipilo

@rootzjg

This comment has been minimized.

Copy link

rootzjg commented Feb 21, 2019

Based on some initial testing I couldn't reproduce the bug anymore. We will do more testing with my team tomorrow and report back. Thank you for your prompt action and help so far.

@mpartipilo

This comment has been minimized.

Copy link

mpartipilo commented Feb 22, 2019

There was some improvement with language switching, but I'm noticing quirks that seem related. Here's how I reproduce the problem:

I have 3 languages (default, chinese and spanish).

  • Create a new collection entry in Chinese
  • Switch to default
  • Top to botton, do "Copy content from: Chinese" on every field
  • When doing it on the Itinerary field the content is not copied.

When saving, closing, re-opening the entry with Default language selected, doing the copy works. Switching to chinese and then back to default, the copy content stops working.

There is nothing printed on the javascript console.

In case it might be useful, here's my collection definition:

<?php
 return array (
  'name' => 'tour',
  'label' => 'Tour',
  '_id' => 'tour5c6d73ce60d7f',
  'fields' => 
  array (
    0 => 
    array (
      'name' => 'name',
      'label' => 'Name',
      'type' => 'text',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => true,
      'acl' => 
      array (
      ),
      'required' => true,
    ),
    1 => 
    array (
      'name' => 'url',
      'label' => 'URL',
      'type' => 'text',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
        'slug' => true,
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
      'required' => true,
    ),
    2 => 
    array (
      'name' => 'heading',
      'label' => 'Heading',
      'type' => 'text',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
      'required' => true,
    ),
    3 => 
    array (
      'name' => 'title',
      'label' => 'Title',
      'type' => 'text',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
    ),
    4 => 
    array (
      'name' => 'category',
      'label' => 'Category',
      'type' => 'collectionlink',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => false,
      'options' => 
      array (
        'link' => 'tourCategory',
        'display' => 'heading',
        'multiple' => false,
        'limit' => false,
      ),
      'width' => '1-1',
      'lst' => true,
      'acl' => 
      array (
      ),
      'required' => true,
    ),
    5 => 
    array (
      'name' => 'price_from',
      'label' => 'Price',
      'type' => 'text',
      'default' => '',
      'info' => 'Prince in Euros',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
    ),
    6 => 
    array (
      'name' => 'duration',
      'label' => 'Duration',
      'type' => 'text',
      'default' => '',
      'info' => 'Duration in days',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
      'required' => true,
    ),
    7 => 
    array (
      'name' => 'short_descr',
      'label' => 'Short Description',
      'type' => 'textarea',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-1',
      'lst' => false,
      'acl' => 
      array (
      ),
    ),
    8 => 
    array (
      'name' => 'content',
      'label' => 'Content',
      'type' => 'markdown',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-1',
      'lst' => false,
      'acl' => 
      array (
      ),
      'required' => false,
    ),
    9 => 
    array (
      'name' => 'itinerary',
      'label' => 'Itinerary',
      'type' => 'markdown',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-1',
      'lst' => false,
      'acl' => 
      array (
      ),
      'required' => true,
    ),
    10 => 
    array (
      'name' => 'inclusions',
      'label' => 'Inclusions',
      'type' => 'markdown',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
    ),
    11 => 
    array (
      'name' => 'exclusions',
      'label' => 'Exclusions',
      'type' => 'markdown',
      'default' => '',
      'info' => '',
      'group' => '',
      'localize' => true,
      'options' => 
      array (
      ),
      'width' => '1-2',
      'lst' => false,
      'acl' => 
      array (
      ),
    ),
  ),
  'sortable' => true,
  'in_menu' => false,
  '_created' => 1550676942,
  '_modified' => 1550763595,
  'color' => '#AC92EC',
  'acl' => 
  array (
  ),
  'rules' => 
  array (
    'create' => 
    array (
      'enabled' => false,
    ),
    'read' => 
    array (
      'enabled' => false,
    ),
    'update' => 
    array (
      'enabled' => false,
    ),
    'delete' => 
    array (
      'enabled' => false,
    ),
  ),
  'group' => 'Tours',
  'icon' => 'paperplane.svg',
  'description' => 'Tour offering',
);
@aheinze

This comment has been minimized.

Copy link
Member

aheinze commented Feb 22, 2019

@mpartipilo so, your issue now is the copy content feature?

@mpartipilo

This comment has been minimized.

Copy link

mpartipilo commented Feb 22, 2019

@aheinze I thought I saw the markdown field displaying the wrong content when switching languages at some point, but I can't reproduce it consistently.

The copy content defect was consistently reproduced following the steps I mentioned, and is perhaps related to your previous fix.

@rootzjg

This comment has been minimized.

Copy link

rootzjg commented Feb 25, 2019

@aheinze my team did a lot of editing on Friday and they did not bump into any problems when switching language. So I think we can be pretty sure that the problem there has been fixed.

They didn't find any side effects either. That I know that they have not used the "copy content" feature as they have been copy pasting already translated content. So currently I have no comments on the problem @mpartipilo has found. I will try and help test that today at some point.

@rootzjg

This comment has been minimized.

Copy link

rootzjg commented Mar 25, 2019

We haven't had any translation issues after the fix @aheinze did. Also the copy content feature is working as expected in our environment. Maybe that one needs more investigating but I think this ticket can be closed as the originally reported problem is resolved.

@mpartipilo

This comment has been minimized.

Copy link

mpartipilo commented Mar 25, 2019

I agree. If I find the problem with copying from another language I'll file an issue specifically for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.