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

Cannot replace root elements in html-fragment #65

Open
shtse8 opened this issue Mar 1, 2017 · 6 comments
Open

Cannot replace root elements in html-fragment #65

shtse8 opened this issue Mar 1, 2017 · 6 comments
Assignees

Comments

@shtse8
Copy link

shtse8 commented Mar 1, 2017

<p>Paragraph 1</p> <p>Paragraph 2</p><p>Paragraph 3</p>

As there is no root, how can I search the p and replace it?
I tired to use filter and replace, but it will remove all my matched elements.

$dom = FluentDOM::QueryCss('<p>Paragraph 1</p> <p>Paragraph 2</p><p>Paragraph 3</p>', 'html-fragment');
$dom->filter('p')->first()->each(function($element) {
	$element->replace('hi');
});
echo $dom;

Expected:

hi <p>Paragraph 2</p><p>Paragraph 3</p>

Actual:

 <p>Paragraph 2</p><p>Paragraph 3</p>
@shtse8 shtse8 changed the title Cannot replace element in html-fragment How can I find the element without root? Mar 1, 2017
@shtse8 shtse8 changed the title How can I find the element without root? Cannot modify elements if there is no root in html-fragment Mar 1, 2017
@shtse8 shtse8 changed the title Cannot modify elements if there is no root in html-fragment Cannot replace root elements in html-fragment Mar 3, 2017
@ThomasWeinert
Copy link
Owner

If you're loading an HTML fragment the top level elements should be automatically selected. Here is a problem with replace() however.

At current implementation required the node to have a parent that is an element node. But in this case it is the document node. I added a test and a fix.

@shtse8
Copy link
Author

shtse8 commented Mar 3, 2017

I have tried the new commit. filter seems can search deeply.

Code:

$dom = FluentDOM::QueryCss('<div><b>5</b><p>4</p></div>', 'html-fragment');
$dom->filter('p')->each(function($element) {
	$element->replace('hi');
});
echo $dom;

Expected:

<div><b>5</b><p>4</p></div>

Actual:

hi

In jquery:

var test = $("<div><b>5</b><p>4</p></div>");
test.filter("p").replaceWith('hi');
console.log(test.outerHTML());

Result:

<div><b>5</b><p>4</p></div>

And with the following Code, the result is also wrong:

$dom = FluentDOM::QueryCss('<p>Paragraph 1</p> <p>Paragraph 2</p><p>Paragraph 3</p><div><b>5</b><p>4</p></div>', 'html-fragment');
$dom->filter('p')->last()->replaceWith('hi');
echo $dom;

Expected:

<p>Paragraph 1</p> <p>Paragraph 2</p>hi<div><b>5</b><p>4</p></div>

Actual:

<p>Paragraph 1</p> <p>Paragraph 2</p><p>Paragraph 3</p>hi

@ThomasWeinert
Copy link
Owner

ThomasWeinert commented Mar 3, 2017

filter() reduces the current selection. You're filtering the current selection for any node that is a 'p' or has a 'p' descendant. The div has a p descendant so it matches. The div is replaced.

Why don't you use find()?

@shtse8
Copy link
Author

shtse8 commented Mar 3, 2017

But the result is different from the jquery version as I posted above.

The reason why I don't use find because I can't find the root elements.

code:

$dom = FluentDOM::QueryCss('<p>Paragraph 1</p> <p>Paragraph 2</p><p>Paragraph 3</p><div><b>5</b><p>4</p></div>', 'html-fragment');
$dom->find('p')->first()->replaceWith('hi');
echo $dom;

Result:

<p>Paragraph 1</p> <p>Paragraph 2</p><p>Paragraph 3</p><div><b>5</b>hi</div>

the first it finds is the p inside the div.

@ThomasWeinert ThomasWeinert reopened this Mar 3, 2017
@ThomasWeinert ThomasWeinert self-assigned this Mar 3, 2017
@shtse8
Copy link
Author

shtse8 commented Mar 19, 2017

Hi, any updates?

ThomasWeinert added a commit that referenced this issue Jun 28, 2017
@ThomasWeinert
Copy link
Owner

The actual problem left seems to be with the CSS Selector to Xpath conversion. I added some tests using specific Xpath expressions and it works:

FluentDOM/FluentDOM@a8c925b

This is difficult to debug because the conversion is not part of FluentDOM itself and an edge case.
I will look into it further but it will take time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants