modxcms / revolution

MODX Revolution - Content Management Framework
https://modx.com/
GNU General Public License v2.0
1.36k stars 529 forks source link

modx-2.6.2-pl - Manager layout problems with non-english language #13854

Closed grafptitsyn closed 6 years ago

grafptitsyn commented 6 years ago

Summary

Manager layout goes crazy when using non-english language, specifically Russian and Deutsch. When Russian language used, top nav bar layout is broken, and almost each left-button mouse click pops up "logout all users" screen.

There is no such problem with modx-2.6.1-pl. Replacing lexicon files with 2.6.1 version doesn't help.

Step to reproduce

  1. Clean install modx-2.6.2-pl, utf8;
  2. Change manager language to "ru" if not configured during installation;
  3. Try to change manager language to "en" by left-button mouse click;
  4. Try to switch resources/elements tabs;
  5. Try left-button mouse click under resources/elements block;

Observed behavior

Environment

MODX Revolution 2.6.2-pl, nginx 1.12.2, php-fpm 5.4.16, mariadb 5.5.56 Browsers: Safari 11.1, Firefox 59.0.2

modx-2 6 2-1 modx-2 6 2-2 modx-2 6 2-3 modx-2 6 2-4

OptimusCrime commented 6 years ago

This sounds very weird. If everyone using a non-english language in the manger had this problem I would guess this problem would have been noticed before and by many more users. I see you have errors in the devtools console, can you post them here?

grafptitsyn commented 6 years ago

@OptimusCrime, Error in devtools console is about missing modx.jsgrps-min.js.map. It's missing OOB also in 2.6.1-pl and doesn't cause the problem.

If you look into second and third screenshots (Russian language), you can see submenu items without title right before "MODX Revolution" label, and devtools shows closing right in href attribute. Fourth screenshot (Deutsch language) shows closing commented causing top nav appear right after "modx-user-menu" element.

At first I thought the problem can be with some non-latin utf8 characters (like umlauts) in lexicon of user-menu items, but overwriting lexicon with 2.6.1-pl won't help. My next thought the problem looks like eval text length limit , that's why closing appears in href attribute when Russian language, and snuck into comment when Deutsch language (total nav characters difference). Btw, menus topnav and usernav cache arrays generated successfully. This pushed me to the idea that the problem is definitely in template rendering.

As I mentioned before, there is no such problem with 2.6.1-pl on the same env:

  1. rm -rf ./*
  2. drop database my_modx_db
  3. unzip modx-2.6.1-pl.zip
  4. clean install modx-2.6.1-pl
  5. All is ok.

UPD: The problem is within Smarty:

  1. Renamed: core/model/smarty -> core/model/smarty_
  2. Copied core/model/smarty from modx-2.6.1-pl
  3. Render problem is gone

UPD2: I just tried another env with PHP 5.3.29 and there is no render problem. This pushed me to idea that Smarty 3.1.31 used in modx-2.6.2-pl is not compatible with PHP 5.4.16. Ok, lets just google for smarty php 5.4 and read about htmlspecialchars()…

Long story short, I modified Smarty.class.php:

if (!defined('SMARTY_RESOURCE_CHAR_SET')) {
    // UTF-8 can only be done properly when mbstring is available!
    /**
    * @deprecated in favor of Smarty::$_CHARSET
    */
    //    define('SMARTY_RESOURCE_CHAR_SET', SMARTY_MBSTRING ? 'UTF-8' : 'ISO-8859-1');
    define('SMARTY_RESOURCE_CHAR_SET', 'ISO-8859-1');
}

That's removed render problem. But I'm sure it's not a bug fix, just a kludge.

opengeek commented 6 years ago

@grafptitsyn are you using an ISO-8859-1 charset? What is your database charset?

grafptitsyn commented 6 years ago

@opengeek, I'm using UTF-8 charset, same DB from one installation.

php.ini:
default_charset = "UTF-8"
mbstring.internal_encoding = UTF-8
MariaDB [(none)]> select * from information_schema.SCHEMATA where SCHEMA_NAME="modx";
+--------------+-------------+----------------------------+------------------------+----------+
| CATALOG_NAME | SCHEMA_NAME | DEFAULT_CHARACTER_SET_NAME | DEFAULT_COLLATION_NAME | SQL_PATH |
+--------------+-------------+----------------------------+------------------------+----------+
| def          | modx        | utf8                       | utf8_unicode_ci        | NULL     |
+--------------+-------------+----------------------------+------------------------+----------+
1 row in set (0.00 sec)
Jako commented 6 years ago

What is your locale? Do you use a non UTF8 capable locale like ru_RU instead of ru_RU.utf8?

grafptitsyn commented 6 years ago

@Jako,

~> locale
LANG=ru_RU.UTF-8
grafptitsyn commented 6 years ago

@OptimusCrime, @opengeek, @Jako, I guess nothing is wrong with my env, because Smarty 3.1.27 has no render problems with PHP 5.4, but Smarty 3.1.31.

Starting from PHP 5.4.0 htmlspecialchars uses UTF-8 encoding by default. Something changed in render mechanism within Smarty 3.1.31, so PHP 5.4 htmlspecialchars behavior began to cause the problem. And that’s why there is no render problem when PHP 5.3.x is used. I have no more ideas what can be wrong.

If I install modx-2.6.2-pl and replace core/model/smarty 3.1.31 with 3.1.27 from modx-2.6.1-pl, then problem is gone. No more changes. Same env, same DB, same locale and DB collation, same ngnix+php-fpm configuration. Just replacing Smarty with previous one. Another “fix” I mentioned before is to redefine SMARTY_RESOURCE_CHAR_SET.

So either I/we should open issue on Smarty, or Server requirements should be updated with no compatibility of modx-2.6.2-pl with PHP 5.4.x, or this “bug” should be fixed by our own. Maybe later I can try to find out which Smarty 3.1.31 method breaks compatibility with PHP 5.4.x and suggest the fix to MODx Community by myself. Should I?

Thanks.

Jako commented 6 years ago

On a local MAMP PRO installation this issue does not occur.

MODX Revolution 2.6.2-pl, Apache, PHP 5.4.45 CGI/FastCGI, MySQL 5.6.38 Browsers: Chrome 65.0.3325.181, Firefox 59.0.2

Now we have to look for the differences. I suppose it is a charset or a locale issue since that produces JSON issues that could disturb the manager scripts. Could you check the PHP locale with var_dump(setlocale(LC_ALL, 0))? Could you set the MODX locale system setting to ru_RU.utf8?

Jako commented 6 years ago

I am quite sure that it is a locale issue, since everything inside of MODX should return UTF8 strings. A not UTF8 capable locale would return ISO encoded strings. But I could be wrong with that.

grafptitsyn commented 6 years ago

@Jako,

var_dump:
string(169)
"LC_CTYPE=ru_RU.utf8;LC_NUMERIC=C;LC_TIME=C;LC_COLLATE=C;LC_MONETARY=C;LC_MESSAGES=C;LC_PAPER=C;LC_NAME=C;LC_ADDRESS=C;LC_TELEPHONE=C;LC_MEASUREMENT=C;LC_IDENTIFICATION=C"
MODX System settings:
locale: ru_RU.UTF-8
modx_charset: UTF-8

Same issue.

Force setting locale via setlocale(LC_ALL,’ru_RU.UTF-8’) in manager/index.php won’t help too.

Jako commented 6 years ago

Does ru_RU.utf8 instead of ru_RU.UTF-8 in the MODX locale system setting makes a difference?

Jako commented 6 years ago

@grafptitsyn According to the var_dump an UTF8 capable locale is set for LC_CTYPE (ru_RU.utf8). The other ones use 'C', which is system specific. Maybe the 'C' locale is not UTF8 capable on your host.

Have you called the var_dump inside of MODX? If yes, the MODX locale system setting is wrong, since it should set the locale to the system setting value. I have prepared a PR that logs such a wrong MODX locale system setting.

grafptitsyn commented 6 years ago

@Jako,

Ouch, I called the var_dump in separate php file. Will try it inside of MODX and then update current comment.

UPD: Snippet code:

<?php
var_dump(setlocale(LC_ALL,0));

Snippet output:

string(11) "ru_RU.UTF-8"

UPD2: Implemented changes you proposed in PR. No errors appeared in log. I cannot stop thinking it’s not an env locale relative problem. Why Smarty 3.1.27 works?

Jako commented 6 years ago

It's a bit difficult to locate your issue on an installation that does not show your issue.

What you could do is to check the HTML of the manager with the different Smarty versions. Some output has to be different. If there is no different output, check the manager scripts that are generated on load (i.e. lexicon script). Locate the differences and we could maybe help you further.

It should be caused by some charset or locale issues.

OptimusCrime commented 6 years ago

What template are you using? Can you make sure you use the default manager template and not one of your custom ones?

grafptitsyn commented 6 years ago

@OptimusCrime, As I mentioned in the first comment, it’s the clean modx install, no modifications at all.

OptimusCrime commented 6 years ago

Oh, sorry.

Jako commented 6 years ago

As I mentioned above, I can't reproduce that issue on my installation on PHP 5.4. So it is your turn to compare the HTML output with the old and the new Smarty version. Without knowing the differences I can't locate the issue.

grafptitsyn commented 6 years ago

@Jako, with new Smarty version HTML output breaks here when Russian language is on: #modx-navbar -> #modx-user-menu -> #limenu-admin -> #modx-subnav -> #lexicon_management Break is at href tag. You can see it on 3rd screenshot.

OptimusCrime commented 6 years ago

Can you please export the entire HTML markup and post it here?

grafptitsyn commented 6 years ago

@OptimusCrime, test.txt

Jako commented 6 years ago

Could you cross check the HTML output with the different Smarty versions. Is it different or equal? If it is different, please post or describe the differences.

Jako commented 6 years ago

In your test.txt there is invalid html code after </li><li id="lexicon_management">

The a tag after has a not closed href attribute.

grafptitsyn commented 6 years ago

@Jako, Yeap, I told about HTML output breakage by Smarty 3.1.31. It can be seen on 3rd screenshot. That's why firstly I started to look into lexicon files of top nav bar. Interchange of them between MODX versions makes no difference, so there is no problem within lexicon files. Then I rolled back Smarty to 3.1.27, and HTML output became correct. And that's why I began to thing the problem is within Smarty renderer, but still I can't find the significant difference between those two versions.

grafptitsyn commented 6 years ago

Here is Smarty 3.1.27 on MODX 2.6.2 output: smarty-3.1.27_output.txt

Jako commented 6 years ago

Maybe there are some somehow wrong encoded signs in the lexicon. In "flush_sessions" is a question mark sign after a truncated line which points in that direction (thats the next part where the output is different). The sign is in the lexicon file topmenu.inc.php in line 43 column 103. There should be an output of й, but a question mark occurs.

grafptitsyn commented 6 years ago

@Jako, modx-2.6.2 + smarty 3.1.31(core/model/smarty) + lexicon-2.6.2(core/lexicon) = broken HTML output modx-2.6.2 + smarty 3.1.31(core/model/smarty) + lexicon-2.6.1(core/lexicon) = broken HTML output modx-2.6.2 + smarty 3.1.27(core/model/smarty) + lexicon-2.6.2(core/lexicon) = correct HTML output

So it's absolutely not lexicon problem.

Btw, I replaced all chars Russian chars with "umlauts" with digits (total 65 chars replaced), and for now broken HTML output around lexicon_management looks like this:

</li><li id="lexicon_management">
<a href="?a=workspac            </ul>

            <ul id="modx-topnav">
                <li id="modx-home-dashboard">
                    <a href="?" title="MODX 2.6.2-pl (traditional)
Панель управления">Панель управления</a>
                </li>

And ?a=workspac appeared in href. Question mark gone to other string position. This question mark tells us string has been truncated, but it shouldn't. Like Smarty 3.1.31 has some output limitations.

smarty-3.1.31-lexicon-ru-umlauts-digits_output.txt

grafptitsyn commented 6 years ago

I think it's much easier to close this ticket and forget about it at this moment…

karpo518 commented 6 years ago

I got the same error with update from 2.2 to 2.6.5 and I can say for sure that it depends on the server software. The error is reproduced on the main server, but is not present on the test server. Downgrade to smarty from MODX 2.6.1 fixes this problem.

ar7n commented 6 years ago

Update smarty to 3.1.33 solves this issue

Jako commented 6 years ago

The new smarty version is merged.