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.3 #109

Closed
joshuabaker opened this issue May 3, 2019 · 18 comments
Closed

PHP 7.3 #109

joshuabaker opened this issue May 3, 2019 · 18 comments
Assignees
Labels

Comments

@joshuabaker
Copy link

I’m seeing some odd output from PHP 7.3.

Code

$settings = new \PHP_Typography\Settings();
$typography = new \PHP_Typography\PHP_Typography();

echo $typography->process('Adipiscing Vehicula Ridiculus Pharetra', $settings);

PHP 7.2 Output

Adipiscing Vehicula Ridiculus Pharetra

PHP 7.3 Output

A d i p ­ i s c ­ i n g V e h i c ­ u ­ l a R i d i c u ­ l u s P h a r e t r a nbsp;
@mundschenk-at
Copy link
Owner

Anything else that might differ between the two versions? I've noticed that the Travis script did not include PHP 7.3, so I rectified that, but the tests run fine.

@mundschenk-at mundschenk-at self-assigned this May 3, 2019
@mundschenk-at
Copy link
Owner

@joshuabaker Can you please be a bit more specific about those two environments (extensions, settings)?

@joshuabaker
Copy link
Author

@mundschenk-at It’s stock MAMP Pro on macOS without anything extra enabled. Specific PHP version is PHP 7.3.1.

@mundschenk-at
Copy link
Owner

@joshuabaker There's something wrong with your PHP installation.

$ php --version
PHP 7.3.5 (cli) (built: May  2 2019 12:40:36) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.5, Copyright (c) 1998-2018 Zend Technologies
    with Xdebug v2.7.0, Copyright (c) 2002-2019, by Derick Rethans
    with Zend OPcache v7.3.5, Copyright (c) 1999-2018, by Zend Technologies

$ cat foo.php 
<?php

require "vendor/autoload.php";

$settings = new \PHP_Typography\Settings();
$typography = new \PHP_Typography\PHP_Typography();

echo htmlentities($typography->process('Adipiscing Vehicula Ridiculus Pharetra', $settings));
echo "\n";

$ php foo.php 
Adip&shy;isc&shy;ing Vehic&shy;u&shy;la Ridicu&shy;lus Pharetra

@joshuabaker
Copy link
Author

I’m getting the same output when using the same version in isolation, directly using MAMP’s PHP.

$ /Applications/MAMP/bin/php/php7.3.1/bin/php --version
PHP 7.3.1 (cli) (built: Feb  1 2019 12:26:46) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.3.1, Copyright (c) 1998-2018 Zend Technologies

$ php test.php
Adip&shy;isc&shy;ing Vehic&shy;u&shy;la Ridicu&shy;lus Pharetra

There’s clearly something going on that’s impacting the library elsewhere in the codebase. Do you have any pointers as to what that might be?

@mundschenk-at
Copy link
Owner

It's pretty weird, looks like something is almost (but not quite) letter-spacing the output. Can you give me some idea what kind of codebase we are talking about? In a WordPress environment, I'd say something is filtering your output, but that's of course not the case here. Maybe some strange parameters for libxml? (Of course html5-php should normalize all that.)

@joshuabaker
Copy link
Author

This is happening on a Craft CMS installation, which itself is based on Yii 2.

@khalwat mentions that there’s others running into the same issue.

@mundschenk-at
Copy link
Owner

mundschenk-at commented May 8, 2019

@joshuabaker Any new findings?

@joshuabaker
Copy link
Author

@mundschenk-at We‘ve put it down for now. Will report back here should we find anything.

@clarknelson
Copy link

I am experiencing the same issue on my website

@mundschenk-at
Copy link
Owner

Are you using Craft as well, @clarknelson?

@clarknelson
Copy link

yes! I am using Craft 3.0.31, and the nystudio107/craft-typogrify package, but "nystudio107" (along with @joshuabaker) have determined that this package is the source of the issue

@mundschenk-at
Copy link
Owner

mundschenk-at commented May 9, 2019

Well no, some kind of interaction with the Craft codebase it would seem. Unfortunately, I know nothing about Craft and cannot reproduce the issue standalone (as documented here). Bring me a minimal test case and I'll gladly fix any bug in PHP-Typography.

@joshuabaker
Copy link
Author

@clarknelson If you read back, this library works fine in PHP 7.3. There’s something happening in the Craft environment that seems to be causing these issues.

@mundschenk-at
Copy link
Owner

@joshuabaker Can you get me a list of installed packages in your Craft environment? It has to be a file that included directly by the autoloader (i.e. containing functions), otherwise this would not happen when you run the minimal example in your Craft directory.

@clarknelson
Copy link

here are my composer dependencies, maybe there is a common package with @joshuabaker

"craftcms/cms": "^3.0.31",
"craftcms/redactor": "^2.1.6",
"craftcms/feed-me": "^4.0",
"verbb/expanded-singles": "^1.0.5",
"verbb/super-table": "^2.0.10",
"vlucas/phpdotenv": "^2.4.0",
"doublesecretagency/craft-cpcss": "^2.1.0",
"nystudio107/craft-seomatic": "^3.1",
"nystudio107/craft-retour": "^3.1",
"nystudio107/craft-typogrify": "^1.1"

@joshuabaker
Copy link
Author

@mundschenk-at Right. Got it. It’s not Craft per se.

Problem Source

Craft is adjusting PHP’s string methods in the web bootstrap file.

setlocale(
    LC_CTYPE,
    'C.UTF-8', // libc >= 2.13
    'C.utf8', // different spelling
    'en_US.UTF-8', // fallback to lowest common denominator
    'en_US.utf8' // different spelling for fallback
);

Test

<?php

require_once 'vendor/autoload.php';

$settings = new \PHP_Typography\Settings();
$typography = new \PHP_Typography\PHP_Typography();


echo htmlentities($typography->process('Adipiscing Vehicula Ridiculus Pharetra', $settings)) . "\n";
// Adip&shy;isc&shy;ing Vehic&shy;u&shy;la Ridicu&shy;lus Pharetra


// Craft’s normalization
setlocale(
    LC_CTYPE,
    'C.UTF-8', // libc >= 2.13
    'C.utf8', // different spelling
    'en_US.UTF-8', // fallback to lowest common denominator
    'en_US.utf8' // different spelling for fallback
);


echo htmlentities($typography->process('Adipiscing Vehicula Ridiculus Pharetra', $settings)) . "\n";
// A&amp;nbsp;d&amp;nbsp;i&amp;nbsp;p&amp;nbsp;&shy;&amp;nbsp;i&amp;nbsp;s&amp;nbsp;c&amp;nbsp;&shy;&amp;nbsp;i&amp;nbsp;n&amp;nbsp;g&amp;nbsp;V&amp;nbsp;e&amp;nbsp;h&amp;nbsp;i&amp;nbsp;c&amp;nbsp;&shy;&amp;nbsp;u&amp;nbsp;&shy;&amp;nbsp;l&amp;nbsp;a&amp;nbsp;R&amp;nbsp;i&amp;nbsp;d&amp;nbsp;i&amp;nbsp;c&amp;nbsp;u&amp;nbsp;&shy;&amp;nbsp;l&amp;nbsp;u&amp;nbsp;s&amp;nbsp;P&amp;nbsp;h&amp;nbsp;a&amp;nbsp;r&amp;nbsp;e&amp;nbsp;t&amp;nbsp;r&amp;nbsp;a&amp;nbsp;

@angrybrad
Copy link

angrybrad commented May 13, 2019

Should be resolved in craftcms/cms@8bf818a, which will be in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants