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.

Sign up for GitHub

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

Jump to bottom

[HOTFIX] Content parser selectors #65

Closed
wants to merge 2 commits into from
Closed

[HOTFIX] Content parser selectors #65

wants to merge 2 commits into from

Conversation

Zamfi99
Copy link
Contributor

@Zamfi99 Zamfi99 commented Jul 9, 2024 •
edited
Loading

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

  1. Check out PR.
  2. Run composer run test.

@Zamfi99 Zamfi99 requested a review from a team as a code owner July 9, 2024 23:25
@ingeniumed
Copy link
Contributor

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

@alecgeatches
Copy link
Contributor

@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 Zamfi99 force-pushed the hotfix/content-parser-selectors branch from 2275d17 to 17c947b Compare July 12, 2024 21:40
@Zamfi99
Copy link
Contributor Author

Zamfi99 commented Jul 12, 2024 •
edited
Loading

@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)

@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 alecgeatches mentioned this pull request Jul 26, 2024
Merged
@alecgeatches alecgeatches mentioned this pull request Jul 30, 2024
Merged
@alecgeatches
Copy link
Contributor

alecgeatches commented Jul 30, 2024 •
edited
Loading

Hello @Zamfi99! I've opened up a second PR based on these changes in #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 alecgeatches mentioned this pull request Jul 30, 2024
Closed
@alecgeatches
Copy link
Contributor

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: #69 (comment). 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Zamfi99 @ingeniumed @alecgeatches