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

PHP 7.1.1 issues (test needed) #2870

Closed
lonalore opened this issue Nov 17, 2017 · 35 comments
Closed

PHP 7.1.1 issues (test needed) #2870

lonalore opened this issue Nov 17, 2017 · 35 comments
Assignees
Labels
type: bug A problem that should not be happening
Milestone

Comments

@lonalore
Copy link
Member

lonalore commented Nov 17, 2017

The following issues are reported by a user on e107hungary forum, I'm just forwarding them:

  1. Cannot be change headlines on news items (I requested more info for this, waiting for response)
    EDIT: Inline-editing on the admin news page is not working under PHP 7.1.1. It jumps to top of the page when clicking on the news title.

  2. Wrong/missing plugin names because e107 cannot find language files (I think we should use english files as fallback, if no language files available for the selected language)

  3. Cannot select sub-forum in the forum:

Provided screenshot for issue 3.
xampp-hiba1

@Jimmi08
Copy link
Contributor

Jimmi08 commented Nov 17, 2017

@lonalore
I confirm that issue with subforum:
#2852

There is what I tried and what helped. But why and how to correctly fix it - no idea.

e107.sk is running on 7.1.6 , so I can help with testing

  • no problems with news in backend
  • if you set this off
    image
    English should be loaded instead missing strings

@tgtje
Copy link
Contributor

tgtje commented Nov 17, 2017

@lonalore is more info (names) available on point 2 ?

Edit : idea, but need more intell...: some plugins are build using core 'structure' of seperate language folders eg. English, Dutch, French etc... while some other plugins use the 'plain structure of having the language files in 'root'mode (no locale folders.. maybe this results in issues having such.. This is an idea i thought about lately, but would need a large testing ground and major rewrites on files to use the lang folders... (hope my thoughts are coming over..)

@Norwayman
Copy link

I totally agree on point 2 :)

@Jimmi08
Copy link
Contributor

Jimmi08 commented Nov 17, 2017

@tgtje or core plugin
image
and by the way. if you have plugin with frontend administration, you still needs double all strings in admin and front file. Working with Lang folders is harder that with all files in one folder languages.
What is point to have folders when lang name is part of file name?
Just my 2 cents.

@tgtje
Copy link
Contributor

tgtje commented Nov 17, 2017

What is point to have folders when lang name is part of file name? <

That is/will be always a point of discussion. I can see a much cleaner system, and as translator can focus on not having to oversee other parts. Suppose you are using 6 or 7 languages and all the needed ones are in 'root', bit messy.... (although should work). Another thought would be when developing. Do i use a seperate locale folder or just my own language (as i believe english should always be present to uphold fallback) > eg.. 1 rule for 'how to'. (some side effects there too)

In long term, i could ( also some tinkering on security) see something (helas no coder and just idea) a system check (ala md5 hash) as a security rule to see if compromise of system (uploaded illegal file(s) folders are part of the system (warning post) etc.... But i stop here.. little going off topic.. (always open for suggestions).

Cause of wrong or missing names. .. That why i ask for more info as i (due to the coding skills i aproach at a different angle... (eg look at it in another way..)

@CaMer0n
Copy link
Member

CaMer0n commented Nov 17, 2017

Thanks @lonalore .
Confirming (3) : Cannot select sub-forum in the forum when PHP 7.1 is active. No problem when PHP 5.6 is in use.

@CaMer0n CaMer0n added the type: bug A problem that should not be happening label Nov 17, 2017
@CaMer0n CaMer0n added this to the e107 v2.1.7 milestone Nov 17, 2017
CaMer0n added a commit that referenced this issue Nov 17, 2017
@lonalore
Copy link
Member Author

I got more info, so I updated the first issue in the list.

@CaMer0n
Copy link
Member

CaMer0n commented Nov 17, 2017

Thanks @lonalore I can't reproduce the first issue on PHP 7.1.1. Perhaps a javascript error somewhere? Could it be related to Hungarian?

@tgtje
Copy link
Contributor

tgtje commented Nov 18, 2017

Small post (if lang for list_new Hung. pack misses if (!defined("PAGE_NAME")) { define("PAGE_NAME", "List New Items"); } in Hung...php; made an issue/post on git/content.... )

edit : (Did a little check on more packs... a lot do miss that line... origin why it was/not in use ?? Bing packs?? Anyway, i exists in English core)
edit 2 : found a issue report telling page_name etc.. should not exist in lang files.... hmmm

@tgtje
Copy link
Contributor

tgtje commented Nov 27, 2017

Ok fresh install of php 7.1.11 (xamp install) No issues on 1
issue1

And on 3 also none

issue3

(in screenshot forgot to add parent (but worked none the less; changed that to First Child on screenshot above)

issue 2 remains unknown (for me)..

@CaMer0n
Copy link
Member

CaMer0n commented Dec 10, 2017

Closing.

@CaMer0n CaMer0n closed this as completed Dec 10, 2017
@lonalore
Copy link
Member Author

@Jimmi08 where is this setting: "Load language files only for current language"? I've been looking for days but can not find it. :D

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 11, 2017

image

@lonalore
Copy link
Member Author

lonalore commented Dec 11, 2017

Ahh thank you! But the current value is 'Off' on my website, but there are some plugins that do not have a hungarian language file. But english files are not included.
@CaMer0n It would be possible to modify e107::lan() to include default, english files if files (for the current language) are not available (or not readable, etc)?

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 11, 2017

Me too. Not sure now. But if I notice nontranslated strings, I just copy English file as Slovak (with lang folder English/English_ this is double work) with English strings. But this haven't happend long time so I supposed that this setting (OFF) works.

@Norwayman
Copy link

languagesettings

the value on my site is also OFF, but if I do not make a folder named Norwegian or Spanish (with the corresponding files in English if I do not have the sufficient time to translate them) it shows just the LAN strings, so that is why I think we should use english files as fallback, if no language files available for the selected language

@Moc Moc reopened this Dec 11, 2017
@CaMer0n
Copy link
Member

CaMer0n commented Dec 11, 2017

@lonalore The code is already there. I am not sure why it is not working for some people. Perhaps some language files are including language files incorrectly?

        public static function includeLan($path, $force = false)
	{
		if (!is_readable($path))
		{
			if (self::getPref('noLanguageSubs') || (e_LANGUAGE === 'English'))
			{
				return false; // pref is "ON" so don't load. 
			}
			
                        // Pref is OFF so load English
			$path = str_replace(e_LANGUAGE, 'English', $path);

			self::getDebug()->log("Couldn't load language file: ".$path);

			if(!is_readable($path))
			{
				return false;
			}
		}
		
		$adminLanguage = self::getPref('adminlanguage');
		
		if(e_ADMIN_AREA && vartrue($adminLanguage))
		{
			$path = str_replace(e_LANGUAGE, $adminLanguage, $path);	
		}
				
		$ret = ($force) ? include($path) : include_once($path);
		return (isset($ret)) ? $ret : "";
	}

@lonalore
Copy link
Member Author

Thank you @CaMer0n I don't use includeLan(), I always use e107::lan(), for example:

// [PLUGINS]/metatag/languages/[LANGUAGE]/[LANGUAGE]_admin.php
e107::lan('metatag', true, true);

And e107::lan() uses e107::coreLan(), e107::themeLan() and e107::plugLan(). But as I see... these methods also use includeLan(). Interesting...

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 11, 2017

@lonalore just to have it complet, this was fixed/changed lately
#2899

@lonalore
Copy link
Member Author

The problem is the missing folder name. So,

../../e107_plugins/metatag/languages/Hungarian_admin.php

should be

../../e107_plugins/metatag/languages/Hungarian/Hungarian_admin.php

kepernyokep_2017-12-11_20-16-39

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 11, 2017

@lonalore but that folder is not mandatory. Both version should be working. I don't use this version for my plugins either.

@lonalore
Copy link
Member Author

So I've always used wrong parameters for e107::lan(). Nice... :/ Mea Culpa!

@lonalore
Copy link
Member Author

OMG....
kepernyokep_2017-12-11_20-33-58

So I've always used wrong parameters for e107::lan() in ALL of my plugins... @%!&!@%!
It will be nice to update all of them...

Anyhow, thank you for the help!

@lonalore
Copy link
Member Author

I am not sure why it is not working for some people.

@CaMer0n It may be because they use e107::lan() the same way I do/did. Or they use my plugins on their websites. :D

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 11, 2017

@lonalore why is your way wrong? Look at documentation: (Maybe I don't see something)

image

@lonalore
Copy link
Member Author

lonalore commented Dec 11, 2017

@Jimmi08 You're right, my solution was based on documentation, but documentation seems to be outdated. I checked PHP docs now, but e107::lan('...', true, true); is not listed here:

/**
 * PREFERRED Generic Language File Loading Function for use by theme and plugin developers. 
 * Language-file equivalent to e107::js, e107::meta and e107::css
 * FIXME disallow themes and plugins named 'core' and 'theme'
 * @param string $type : 'theme' or plugin name
 * @param $string $fname (optional): relative path to the theme or plugin language folder. (same as in the other functions)
 * when missing, [e_LANGUAGE]_front.php will be used, when true [e_LANGUAGE]_admin.php will be used
 * @param $options : Set to True for admin. 
 * @example e107::lan('theme'); // Loads THEME."languages/English.php (if English is the current language)
 * @example e107::lan('gallery'); // Loads e_PLUGIN."gallery/languages/English_front.php (if English is the current language)
 * @example e107::lan('gallery', 'admin'); // Loads e_PLUGIN."gallery/languages/English/admin.php (if English is the current language)
 * @example e107::lan('gallery', 'admin', true); // Loads e_PLUGIN."gallery/languages/English_admin.php (if English is the current language)
 * @example e107::lan('gallery', 'admin/example'); // Loads e_PLUGIN."gallery/languages/English/admin/example.php (if English is the current language)
 * @example e107::lan('gallery', true); // Loads e_PLUGIN."gallery/languages/English_admin.php (if English is the current language)
 * @example e107::lan('gallery', "something", true); // Loads e_PLUGIN."gallery/languages/English_something.php (if English is the current language)
 */
public static function lan($type, $fname = null, $options = null)
{
	...
}

@lonalore
Copy link
Member Author

I became uncertain now... I don't know which documentation is the good one.

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 11, 2017

I am lost too, there is "break" in logic/system in these examples above.

There should be strict rules and plugins develop should use them.
I use now:

e107::lan('creative_writer',true);
e107::lan('creative_writer');

So many possibilities and no one what would be load simple English.php if this is because back compatibility.

@lonalore
Copy link
Member Author

lonalore commented Dec 11, 2017

My (current, working) implementations:

// [PLUGINS]/metatag/languages/[LANGUAGE]_front.php
e107::lan('metatag');

// [PLUGINS]/metatag/languages/[LANGUAGE]_admin.php
e107::lan('metatag', 'admin', true);

Maybe e107::lan('...', true, true); worked before, but it may have broken somewhere... :/

I think it would be a great need to update outdated PHP docs and it would also be a great need to write unit/functional tests for e107 to avoid such misunderstandings.

@rica-carv
Copy link
Contributor

rica-carv commented Dec 12, 2017

Maybe i'm wrong, but e107::lan('...', true, true); was on the earlyer v2. It was a short lived option, i think...
I've also had that "bug", and had to adapt all my plugins to the new way....

@lonalore
Copy link
Member Author

It is interesting that e107::lan('metatag', true, true); loads [PLUGINS]/metatag/languages/English/English_admin.php file if the current language is English. But it tries to load [PLUGINS]/metatag/languages/Hungarian_admin.php for hungarian language. I think this is a bug, and the documentation is good.

@tgtje
Copy link
Contributor

tgtje commented Dec 12, 2017

Just adding : It is the 'problem ' i mentioned before (Gitter??) here is no straight rule if files must be on 'root', or inside a locale (country) folder. Where one works with locale files, and others with 'root ' files ('root'= main language folder). The mix of both is throughout.

@rica-carv
Copy link
Contributor

I think the two options (inside and outside country folder) are there because of legacy complatibility.
Old V1.x plugins and themes don't have the country folder, so the code is that way to make them work too...

@Jimmi08
Copy link
Contributor

Jimmi08 commented Dec 13, 2017

@rica-carv for legacy it would be there option load languages/English.php file. This is not there as option.

Using folder and language in file name is duplicity. You have no such case in these examples or I am blind.

This is correct: /English/admin.php or this: /English/admin/example.php but not this /English/English_admin.php.

* @example e107::lan('gallery'); // Loads e_PLUGIN."gallery/languages/English_front.php (if English is the current language)
 * @example e107::lan('gallery', 'admin'); // Loads e_PLUGIN."gallery/languages/English/admin.php (if English is the current language)
 * @example e107::lan('gallery', 'admin', true); // Loads e_PLUGIN."gallery/languages/English_admin.php (if English is the current language)
 * @example e107::lan('gallery', 'admin/example'); // Loads e_PLUGIN."gallery/languages/English/admin/example.php (if English is the current language)
 * @example e107::lan('gallery', true); // Loads e_PLUGIN."gallery/languages/English_admin.php (if English is the current language)
 * @example e107::lan('gallery', "something", true); // Loads e_PLUGIN."gallery/languages/English_something.php (if English is the current language)

This was referenced Dec 13, 2017
@lonalore
Copy link
Member Author

The problem was: is_dir(e_PLUGIN.$plugin."/languages/".e_LANGUAGE) in the condition. Because the (Hungarian) folder does not exist if no translated files, so the condition always returned with false, and the $path = e_PLUGIN.$plugin.'/languages/'.$fname.'.php'; was used instead of the proper path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A problem that should not be happening
Projects
None yet
Development

No branches or pull requests

7 participants