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

Warnings raised in AMP_Style_Sanitizer::get_validate_response_data() due to nodes no longer existing #5169

Closed
westonruter opened this issue Aug 5, 2020 · 1 comment
Labels
Bug Something isn't working CSS Validation WS:Core Work stream for Plugin core

Comments

@westonruter
Copy link
Member

Bug Description

When activating this gist plugin which is rendering an amp-story without the required standalone attribute (resulting in the entire amp-story being removed), there are warnings being raised by code in AMP_Style_Sanitizer::get_validate_response_data():

PHP Warning:  Couldn't fetch DOMElement. Node no longer exists in …/amp/includes/sanitizers/class-amp-style-sanitizer.php on line 2924
PHP Notice:  Undefined property: DOMElement::$attributes in …/amp/includes/sanitizers/class-amp-style-sanitizer.php on line 2924
PHP Warning:  Invalid argument supplied for foreach() in …/amp/includes/sanitizers/class-amp-style-sanitizer.php on line 2924
PHP Warning:  Couldn't fetch DOMElement. Node no longer exists in …/amp/includes/sanitizers/class-amp-style-sanitizer.php on line 2928
PHP Notice:  Undefined property: DOMElement::$nodeName in …/amp/includes/sanitizers/class-amp-style-sanitizer.php on line 2928

Line 2924:

foreach ( $pending_stylesheet['element']->attributes as $attribute ) {

Line 2928:

'name' => $pending_stylesheet['element']->nodeName,

This is strange to me because the original DOMElement object is being stored in the pending_stylesheets array. Somehow the reference is going bad.

Expected Behaviour

No warnings should be emitted when the style sanitizer is iterating over the pending_stylesheets when elements are removed from the document.

Steps to reproduce

  1. Enable AMP on all templates (e.g. in Standard mode)
  2. Activate the gist plugin
  3. Access a URL that would normally be an AMP page.
  4. Now add ?try_node_no_longer_exists_warning=1
  5. Click Validate URL in the admin bar.
  6. See warnings in the error log.

Screenshots

In the Stylesheets metabox you'll also see:

image

The title attribute value is <>.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter westonruter added Bug Something isn't working Validation CSS labels Aug 5, 2020
@kmyram kmyram added the WS:Core Work stream for Plugin core label Aug 5, 2020
@westonruter
Copy link
Member Author

Related: #3393.

@westonruter westonruter closed this as not planned Won't fix, can't repro, duplicate, stale Nov 30, 2023
@github-project-automation github-project-automation bot moved this to Backlog in Ongoing Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working CSS Validation WS:Core Work stream for Plugin core
Projects
Archived in project
Development

No branches or pull requests

2 participants