-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Use internal libxml errors instead of @suppression #24677
Conversation
Instead of using `@` to suppress errors during `DOMDocument::loadHTML` use `libxml_use_internal_errors` to capture and suppress the errors instead. If there is a custom `set_error_handler` set for your installation it will continue to receive warnings and html validation errors from this `loadHTML` call. If the goal is to suppress these errors we should try to capture and drop these without polluting customized error logs.
@ockham may want to take a look at this too - I'm not totally sure what the original suppression was for but I'm hoping this is capturing all those same errors. |
} | ||
); | ||
|
||
// HTML5 elemetns like <time> are not supported by the DOMDocument parser used by the block supports feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// HTML5 elemetns like <time> are not supported by the DOMDocument parser used by the block supports feature. | |
// HTML5 elements like <time> are not supported by the DOMDocument parser used by the block supports feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good, thanks for the fix! One typo, otherwise good to merge (once GH Actions go all green).
Reminder: I don't have merge rights here. |
Merged. Thanks again @lsl! @jorgefilipecosta @youknowriad @mcsf Can we maybe include this in 8.9? These warnings can otherwise clutter warning logs (or the frontend, if WP_DEBUG is true). |
Instead of using `@` to suppress errors during `DOMDocument::loadHTML`, use `libxml_use_internal_errors` to capture and suppress the errors instead. If there is a custom `set_error_handler` set for your installation, it will continue to receive warnings and html validation errors from this `loadHTML` call. If the goal is to suppress these errors we should try to capture and drop these without polluting customized error logs.
Seems like all warnings are not suppressed yet https://wordpress.org/support/topic/warning-a-non-numeric-value-encountered-31/#post-13357203 I wonder if we should try to avoid DOMDocument and just use Regex to include the classes and styles like explored here https://github.com/WordPress/gutenberg/pull/21420/files#diff-4339a9e86722fd029c231f3fb65f0ad2R394-R412 |
Description
Instead of using
@
to suppress errors duringDOMDocument::loadHTML
use
libxml_use_internal_errors
to capture and suppress the errorsinstead.
If there is a custom
set_error_handler
set for your installation itwill continue to receive warnings and html validation errors from this
loadHTML
call. If the goal is to suppress these errors we should tryto capture and drop these without polluting customized error logs.
How has this been tested?
@
suppressionTypes of changes
Bug fix, there shouldn't be any breaking changes although some people may see less errors for broken block html or html5 features passing through this code.
Checklist: