8000 Impossible to select non-DOMElements · Issue #16933 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Impossible to select non-DOMElements #16933

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
Closed
EdgarPE opened this issue Dec 9, 2015 · 6 comments
Closed

Impossible to select non-DOMElements #16933

EdgarPE opened this issue Dec 9, 2015 · 6 comments
Labels
DomCrawler

Comments

@EdgarPE
Copy link
Contributor
EdgarPE commented Dec 9, 2015

This patch: 9f362a1 introduced a serious limitation, namely one can not select non-DOMElements. For example, now it is impossible to select only the text content of a specific node, like this: $node->filterXPath('//text()')
The selected node never gets returned because of this:

if ($subNode->nodeType === 1) {
    $crawler->add($subNode);
}

I suggest a solution, where adding non-DOMElements are prevented but selecting one is possible. Is the community open for that? If yes, I am willing to work on it.
@jakzal
Copy link
Contributor
jakzal commented Dec 10, 2015

@EdgarPE Filtered nodes are added to a new crawler instance. That's the reason why the patch you referenced mentions "Prevent adding non-DOMElement elements in DomCrawler". We can't implement what you're asking for.

We could try to throw an exception if someone tries to use a non-DOMElement in a wrong context (in a form for example). I haven't looked how complex this could be, and it might be too late for this change now.

See #16058 for reasons why this change was introduced.

@EdgarPE EdgarPE mentioned this issue Dec 15, 2015
Closed
@EdgarPE
Copy link
Contributor Author
EdgarPE commented Dec 15, 2015

@jakzal Thank you, for your insights. Based on your comment I just made a Pull Request: #17021

@armpogart
Copy link

So is it expected behavior that now I can do $node->filterXPath('//div') but not $node->filterXPath('//div/text()'). My application was using /text() xpath selector intensively, what are you suggesting - to use older version? Will full xpath syntax be supported in any future version?

@EdgarPE
Copy link
Contributor Author
EdgarPE commented Dec 22, 2015

Hopefully the Pull request will be merged soon. If you need to update to 2.8 or 3.0 now, one workaround is to select the supported node (//div), get the DOMElement object, then use xpath to filter the DOMText object. It's not ideal, but it works.

@armpogart
Copy link

Thanks for your quick response, I will be glad if you also show me how to get the DOMElement object and use xpath on it. I don't get it, isn't filtered node always of type Symfony\Component\DomCrawler\Crawler? I couldn't find any way there to get the DOMElement.

Also does that mean if your PR will be merged /text() will work on 2.8 and 3.* also or 3.* will break the compatibility?

@EdgarPE EdgarPE mentioned this issue Dec 22, 2015
Closed
@stof
Copy link
Member
stof commented Dec 22, 2015

@armpogart ->getNode(0) gives you the underlying DOM node (change the argument to get other indexes of course). And the iterator also gives you DOM nodes.

jakzal added a commit that referenced this issue Dec 23, 2015
@jakzal
feature #17035 [DomCrawler] Revert previous restriction, allow select…
09efd06 
…ion of every DOMNode object (EdgarPE)

This PR was squashed before being merged into the 2.8 branch (closes #17035).

Discussion
----------

[DomCrawler] Revert previous restriction, allow selection of every DOMNode object

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes, revert to previous behaviour
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #16933
| License       | MIT
| Doc PR        |

This is a backport of PR #17021

Commits
-------

d2872a3 [DomCrawler] Revert previous restriction, allow selection of every DOMNode object
@jakzal jakzal closed this as completed Dec 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
DomCrawler
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jakzal @EdgarPE @stof @armpogart @xabbuh
0