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

BUGFIX: Prevent augmenter from applying data of multiple nodes into the same element #3856

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

Sebobo
Copy link
Member

@Sebobo Sebobo commented Sep 25, 2024

What I did

This fixes a regression introduced in b56135a which removed the script tag, causing the augmenter to add the data of multiple nodes into the same html element in certain cases instead of adding an outer div. This lead to an error with the FlowQuery JavaScript API as it couldn't handle two nodepaths separated by a space as a single nodepath.

With this change the augmenter automatically adds an outer div if a node attribute appears twice. Therefore we have a single element for each wrapped node.

How to verify it

Create the following nodetype in the demo package:

NodeType:

Neos.Demo:Content.Test:
  ui:
    label: 'Test'
  superTypes:
    'Neos.Neos:Content': true
    'Neos.Demo:Constraint.Content.Main': true
  childNodes:
    test:
      type: 'Neos.Neos:ContentCollection'

Fusion prototype:

prototype(Neos.Demo:Content.Test) < prototype(Neos.Neos:ContentComponent) {
    renderer = afx`
        <Neos.Neos:ContentCollection nodePath="test"/>
    `
}

In 8.3.10 the rendered node in the backend would have two values in its context-path and fusion-path attributes and the FlowQuery endpoint throws an error when loading.

With the change the backend works fine again as in 8.3.9 and parent and child node are selectable.

@Sebobo Sebobo added Bug Label to mark the change as bugfix 8.3 labels Sep 25, 2024
…he same element

This fixes a regression introduced in #b56135a01ecf59ae3a4990e3fd54ac766732e0e6 which
removed the script tag, causing the augmenter to add the data of multiple nodes into the same
html element in certain cases instead of adding an outer div.

With this change this behaviour is now more explicit instead of relying on the inner workings
of the augmenter.
@Sebobo Sebobo force-pushed the bugfix/enforce-dom-node-for-augmentation branch from cbf46ef to 0b964bd Compare September 25, 2024 10:49
Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks seems like the most clean and correct solution for this case.

@kitsunet kitsunet merged commit 00d94b6 into 8.3 Sep 25, 2024
11 checks passed
@kitsunet kitsunet deleted the bugfix/enforce-dom-node-for-augmentation branch September 25, 2024 19:16
@mhsdesign
Copy link
Member

I cannot upmerge this fix to Neos 9 as the class was moved to Neos.Neos so this change has to be redone there!

mhsdesign added a commit to mhsdesign/neos-development-collection that referenced this pull request Oct 8, 2024
…he same element

9.0 port of neos/neos-ui#3856

Original Commit MSG:

This fixes a regression introduced in #b56135a01ecf59ae3a4990e3fd54ac766732e0e6 which
removed the script tag, causing the augmenter to add the data of multiple nodes into the same
html element in certain cases instead of adding an outer div.

With this change this behaviour is now more explicit instead of relying on the inner workings
of the augmenter.
@mhsdesign
Copy link
Member

neos-bot pushed a commit to neos/neos that referenced this pull request Oct 9, 2024
…he same element

9.0 port of neos/neos-ui#3856

Original Commit MSG:

This fixes a regression introduced in #b56135a01ecf59ae3a4990e3fd54ac766732e0e6 which
removed the script tag, causing the augmenter to add the data of multiple nodes into the same
html element in certain cases instead of adding an outer div.

With this change this behaviour is now more explicit instead of relying on the inner workings
of the augmenter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.3 Bug Label to mark the change as bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants