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

Resolve deprecations #424

Closed
g105b opened this issue Feb 16, 2023 · 12 comments · Fixed by #439
Closed

Resolve deprecations #424

g105b opened this issue Feb 16, 2023 · 12 comments · Fixed by #439
Projects

Comments

@g105b
Copy link
Member

g105b commented Feb 16, 2023

In PHP 8.2, the following deprecations are emitted:

Deprecated: mb_convert_encoding(): Handling HTML entities via mbstring is deprecated; use htmlspecialchars, htmlentities, or mb_encode_numericentity/mb_decode_numericentity instead in /app/vendor/phpgt/dom/src/Element.php on line 164
@g105b g105b added this to Backlog in Overview via automation Feb 16, 2023
@g105b
Copy link
Member Author

g105b commented Feb 16, 2023

There are many solutions online, but none of them seem to appreciate that some developers put 🌮 emoji 🥕 in their HTML, or even simple things like "Polski jest pięknym językiem".

My solution:

$html = mb_encode_numericentity(
	$html,
	[0x80, 0x10FFFF, 0, -1],
	"UTF-8"
);

@glo71317
Copy link

Hello @g105b,

I hope you are doing good!

I have a query regarding this issue, there is any plan to fixed in upcoming near because we are planing to use latest version of "phpgt/dom" in magento side. In last release we have provided the fix for the same - #412 and you had released v2 new release and we are using "phpgt/dom":"^2.2.4".

Can you please have a look and help us in this regard?

@g105b
Copy link
Member Author

g105b commented Jul 17, 2023

Hi @glo71317,

I'm doing great, thanks. Hope everything is going well with you too.

It's fantastic to hear Magento are switching to the latest version. There are plenty of nice new features, and performance increases since v2.

The issue #412 was fixed as a backport to v2 to support users of that version. There are tests covering multi-byte strings and unicode characters, and I've just added the specific mb_encode_numericentity fix to the HTMLElement class in a PR.

Please can you test your code with the 424-html-entities branch (PR here), and please can you cast your attention to the escaped characters unit tests?

Let me know how you get on. If there's any tweaks the repository needs, please share your findings here and I'll happily apply any fixes to help you get on to the latest version.

g105b added a commit that referenced this issue Jul 17, 2023
@glo71317
Copy link

@g105b Thanks lot for quick reply!

I will have a look and will update you tomorrow.

@glo71317
Copy link

@g105b I have reviewed your PR changes, Changes are looks fine to me and also you can keep escaped characters unit tests which are required to test some scenarios.

I have one question regarding this change - https://github.com/PhpGt/Dom/blob/v4.1.2/src/HTMLDocument.php#L36 because we had delivered https://github.com/PhpGt/Dom/blob/v2.2.4/src/HTMLDocument.php#L35C1-L40 which is tested at our end properly.

Can you please confirm about this changes - https://github.com/PhpGt/Dom/blob/v4.1.2/src/HTMLDocument.php#L36 is tested at your end or not?

Note - Testing in progress with your PR changes, we are facing some issue once it will be complete i will notify you.

@g105b
Copy link
Member Author

g105b commented Jul 18, 2023

The loadHTML functionality, with the flags LIBXML_SCHEMA_CREATE | LIBXML_COMPACT have been tested for unicode/utf-8 support, but please see if you can find any edge cases that the unit tests are not covering. I can not find any cases where loading multibyte characters is escaped badly or malformed.

Keep me updated and have a good day.

Overview automation moved this from Backlog to Done July 2023 Jul 20, 2023
g105b added a commit that referenced this issue Jul 20, 2023
* fix: use mb_encode_numericentity for #424

* test: add copyright-trademark test to Element

* fix: use mb_encode_numericentity for #424

* test: add copyright-trademark test to Element
@g105b
Copy link
Member Author

g105b commented Jul 20, 2023

Hi @glo71317,

I've just made a new patch release to the library: https://github.com/PhpGt/Dom/releases/tag/v4.1.3

This includes the aforementioned PR, and some rather small changes.

Please feed back your experience with this release and I'll be here if you need me.

Cheers,
Greg.

@glo71317
Copy link

@g105b Thanks for releasing the new patch.

I am working on BIC issue after updating the latest version of "phpgt/dom". So it may required time to resolve.

Surely, i will catch you if any help will require.

Thanks,
Rajesh

@g105b
Copy link
Member Author

g105b commented Jul 21, 2023

Great. Feel free to reach out whenever you need any help. I'd like to hear about your success too, not just anything negative 😄

Speak soon and have fun!

@glo71317
Copy link

Hello @g105b

Greeting for the day!

I had tried to update the latest version- "phpgt/dom": "^4.1.3" which is released few days back.
When i am updating latest version, i am not able to update the latest version because dependency of "psr/http-message" changed from ^1.0 to ^2.0 in "phpgt/dom": "^4.1.3".

From Magento side, several third party dependency are using "psr/http-message": "^1.0"
Screenshot 2023-07-24 at 7 10 37 PM
Screenshot 2023-07-24 at 7 08 54 PM

So, i don't know it will be possible or not from your end.
if this dependency https://github.com/PhpGt/Dom/blob/v4.1.3/composer.json#L13, we can change e.g. "psr/http-message": "^1.0 || ^2.0" then it should be resolve the issue.

@g105b
Copy link
Member Author

g105b commented Jul 24, 2023

Thanks for letting me know. There's nothing Dom relies on in 2.0 that can't be satisfied by 1.0, so I'll get that changed and make a patch release for you.

@glo71317
Copy link

Great and Thanks to you for understanding!

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

Successfully merging a pull request may close this issue.

2 participants