-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix(selector): normalise whitespaces in text selector #3873
fix(selector): normalise whitespaces in text selector #3873
Conversation
@@ -118,7 +118,7 @@ function queryInternal(root: SelectorRoot, matcher: Matcher, shadow: boolean): E | |||
if (!node) | |||
break; | |||
if (node.nodeType === Node.TEXT_NODE) { | |||
lastText += node.nodeValue; | |||
lastText += node.nodeValue ? node.nodeValue.replace(/\s/g, ' ') : node.nodeValue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's instead update matcher
to cover element.value
check below, and also similar checks in queryAllInternal.
@@ -94,6 +94,9 @@ it('query', async ({page, isWebKit}) => { | |||
}); | |||
expect(await page.$eval(`text=lowo`, e => e.outerHTML)).toBe('<div>helloworld</div>'); | |||
expect(await page.$$eval(`text=lowo`, els => els.map(e => e.outerHTML).join(''))).toBe('<div>helloworld</div><span>helloworld</span>'); | |||
|
|||
await page.setContent('<button id="testAttribute">Sign in</button>'); | |||
expect(await page.$eval('text="Sign in"', e => e.id)).toBe('testAttribute'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a test for $$
or $$eval
.
@@ -94,6 +94,9 @@ it('query', async ({page, isWebKit}) => { | |||
}); | |||
expect(await page.$eval(`text=lowo`, e => e.outerHTML)).toBe('<div>helloworld</div>'); | |||
expect(await page.$$eval(`text=lowo`, els => els.map(e => e.outerHTML).join(''))).toBe('<div>helloworld</div><span>helloworld</span>'); | |||
|
|||
await page.setContent('<button id="testAttribute">Sign in</button>'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we now match '\n' as well? Let's add a test. Also, do we want to collapse spaces, e.g. match "\n \n" by a single space? Maybe only in the relaxed mode without quotes?
Superseded by #4312 |
Fixes #3830