[HOTFIX] Content parser selectors

Zamfi99 commented 3 months ago

Description

For some specific use cases, such as retrieving the anchor attributes from the HTML of every block, the API may unexpectedly return null.

For example:

If the universal CSS selector is used (*) then the first node returned by $crawler->filter( $selector ); will be the body element that doesn't contain any attribute, so the $crawler->attr( $attribute ); will always return null.

To avoid this outcome, we can modify the source_block_attribute method in order to filter the elements within the body element.

Steps to Test

Check out PR.
Run composer run test.

ingeniumed commented 3 months ago

Thanks for making this PR @Zamfi99, we will look into this fix 👍🏾

alecgeatches commented 3 months ago

@Zamfi99 Thank you for this report! Can you give an example of markup where you see this issue, so we can understand it better? Thank you! Here's a template you can use:

Gutenberg Markup

<!-- wp:paragraph -->
<p>Some example HTML from the block editor...</p>
<!-- /wp:paragraph -->

Block Data API Result

[{
  "name": "core/paragraph",
  "attributes": {
    "content": "Some example HTML from the block editor..."
  }
}]

Expected Result

(what you expected to see)

Zamfi99 commented 3 months ago

@Zamfi99 Thank you for this report! Can you give an example of markup where you see this issue, so we can understand it better? Thank you! Here's a template you can use:

Gutenberg Markup

<p>Some example HTML from the block editor...</p>

Block Data API Result
[{
  "name": "core/paragraph",
  "attributes": {
    "content": "Some example HTML from the block editor..."
  }
}]
Expected Result
(what you expected to see)

@alecgeatches Sure thing. I've also made some changes and updated the PR description.

Gutenberg Markup

<!-- wp:image {"id":563,"sizeSlug":"large","linkDestination":"none","sfsBlockId":"811c30a5-1213-440f-a2a9-ee9ec3b4936e","className":"additional-css-class","credit":"","orientationOnMobile":"vertical"} -->
<figure class="wp-block-image size-large additional-css-class" id="anchor">
    <img src="https://example.com" alt="alt" class="wp-image-563" title="title"/>
    <figcaption class="wp-element-caption">Caption.</figcaption>
</figure>
<!-- /wp:image -->

Block Data API Result

[
  {
    "id": "QmxvY2tEYXRhOjcxNDox",
    "name": "core/image",
    "attributes": [
      {
        "name": "id",
        "value": "563",
        "isValueJsonEncoded": true
      },
      // removed the other attributes
      {
        "name": "anchor",
        "value": "",
        "isValueJsonEncoded": false
      }
    ],
    "innerBlocks": null
  }
]

Expected Result

[
  {
    "id": "QmxvY2tEYXRhOjcxNDox",
    "name": "core/image",
    "attributes": [
      {
        "name": "id",
        "value": "563",
        "isValueJsonEncoded": true
      },
      // removed the other attributes
      {
        "name": "anchor",
        "value": "anchor",
        "isValueJsonEncoded": false
      }
    ],
    "innerBlocks": null
  }
]

Clarification

By running wp.blocks.getBlockTypes() in the Gutenberg editor, below is the anchor attribute definition for the core/image block.

[
    {
        "anchor": {
            "attribute": "id",
            "selector": "*",
            "source": "attribute",
            "type": "string"
        }
    }
]

alecgeatches commented 3 months ago

Hello @Zamfi99! I've opened up a second PR based on these changes in https://github.com/Automattic/vip-block-data-api/pull/71. The main differences:

<body> is entered right at the top of parsing. Your code here makes that adjustment for attribute sources specifically, but the change in the new PR should cover more cases where the <body> element is unnecessarily part of parsing.
Because of the generalized <body> change, needed to adjust the raw source type to work properly.
Added a test specifically for the anchor case.

If you have time, feel free to take a look at those changes before they're merged and ensure they address your issue. I'll plan to merge them in tomorrow.

alecgeatches commented 2 months ago

I'm going to close this issue for now. We've added a fix for * selectors in #71, and provided a workaround for anchor and ariaLabel block support attributes here: https://github.com/Automattic/vip-block-data-api/issues/69#issuecomment-2263535285. Please see the linked comment's issue for more explanation on why we're not making anchor support a default behavior, but it's fairly easy to work around.

Thank you for your input and feedback in this and related issues!

Automattic / vip-block-data-api