From 5f527fedb1f6893219b69d735b1a9cdd81ad1466 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 9 Nov 2023 00:11:01 +0100 Subject: [PATCH] fix: JSHandle preview text for non-ascii attributes/children (#28038) This surfaced in .NET that the string in the driver got incorrectly cut, then transferred to .NET as an invalid UTF8 character [`\ud835`](https://charbase.com/d835-unicode-invalid-character) which .NET wasn't able to parse and threw an error. Drive-by: Move similar function from `packages/playwright-core/src/client/page.ts` into isomorphic `stringUtils`. https://github.com/microsoft/playwright-dotnet/issues/2748 --- packages/playwright-core/src/client/page.ts | 11 +++-------- .../src/server/injected/injectedScript.ts | 12 ++++-------- .../src/server/injected/selectorGenerator.ts | 4 ++-- .../src/utils/isomorphic/stringUtils.ts | 13 +++++++++++++ tests/page/elementhandle-convenience.spec.ts | 7 +++++++ tests/page/page-wait-for-request.spec.ts | 2 +- 6 files changed, 30 insertions(+), 19 deletions(-) diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index 0e8701f905..0430e3c0bc 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -42,6 +42,7 @@ import { Keyboard, Mouse, Touchscreen } from './input'; import { assertMaxArguments, JSHandle, parseResult, serializeArgument } from './jsHandle'; import type { FrameLocator, Locator, LocatorOptions } from './locator'; import type { ByRoleOptions } from '../utils/isomorphic/locatorUtils'; +import { trimStringWithEllipsis } from '../utils/isomorphic/stringUtils'; import { type RouteHandlerCallback, type Request, Response, Route, RouteHandler, validateHeaders, WebSocket } from './network'; import type { FilePayload, Headers, LifecycleEvent, SelectOption, SelectOptionOptions, Size, URLMatch, WaitForEventOptions, WaitForFunctionOptions } from './types'; import { Video } from './video'; @@ -751,15 +752,9 @@ export class BindingCall extends ChannelOwner { } } -function trimEnd(s: string): string { - if (s.length > 50) - s = s.substring(0, 50) + '\u2026'; - return s; -} - function trimUrl(param: any): string | undefined { if (isRegExp(param)) - return `/${trimEnd(param.source)}/${param.flags}`; + return `/${trimStringWithEllipsis(param.source, 50)}/${param.flags}`; if (isString(param)) - return `"${trimEnd(param)}"`; + return `"${trimStringWithEllipsis(param, 50)}"`; } diff --git a/packages/playwright-core/src/server/injected/injectedScript.ts b/packages/playwright-core/src/server/injected/injectedScript.ts index 48a9a388dd..13a35cdb53 100644 --- a/packages/playwright-core/src/server/injected/injectedScript.ts +++ b/packages/playwright-core/src/server/injected/injectedScript.ts @@ -33,7 +33,7 @@ import { getChecked, getAriaDisabled, getAriaRole, getElementAccessibleName } fr import { kLayoutSelectorNames, type LayoutSelectorName, layoutSelectorScore } from './layoutSelectorUtils'; import { asLocator } from '../../utils/isomorphic/locatorGenerators'; import type { Language } from '../../utils/isomorphic/locatorGenerators'; -import { normalizeWhiteSpace } from '../../utils/isomorphic/stringUtils'; +import { normalizeWhiteSpace, trimStringWithEllipsis } from '../../utils/isomorphic/stringUtils'; type Predicate = (progress: InjectedScriptProgress) => T | symbol; @@ -1072,9 +1072,7 @@ export class InjectedScript { attrs.push(` ${name}="${value}"`); } attrs.sort((a, b) => a.length - b.length); - let attrText = attrs.join(''); - if (attrText.length > 50) - attrText = attrText.substring(0, 49) + '\u2026'; + const attrText = trimStringWithEllipsis(attrs.join(''), 50); if (autoClosingTags.has(element.nodeName)) return oneLine(`<${element.nodeName.toLowerCase()}${attrText}/>`); @@ -1085,10 +1083,8 @@ export class InjectedScript { for (let i = 0; i < children.length; i++) onlyText = onlyText && children[i].nodeType === Node.TEXT_NODE; } - let text = onlyText ? (element.textContent || '') : (children.length ? '\u2026' : ''); - if (text.length > 50) - text = text.substring(0, 49) + '\u2026'; - return oneLine(`<${element.nodeName.toLowerCase()}${attrText}>${text}`); + const text = onlyText ? (element.textContent || '') : (children.length ? '\u2026' : ''); + return oneLine(`<${element.nodeName.toLowerCase()}${attrText}>${trimStringWithEllipsis(text, 50)}`); } strictModeViolationError(selector: ParsedSelector, matches: Element[]): Error { diff --git a/packages/playwright-core/src/server/injected/selectorGenerator.ts b/packages/playwright-core/src/server/injected/selectorGenerator.ts index 51ff0c2560..e353ad1556 100644 --- a/packages/playwright-core/src/server/injected/selectorGenerator.ts +++ b/packages/playwright-core/src/server/injected/selectorGenerator.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalizeWhiteSpace, quoteCSSAttributeValue } from '../../utils/isomorphic/stringUtils'; +import { cssEscape, escapeForAttributeSelector, escapeForTextSelector, normalizeWhiteSpace, quoteCSSAttributeValue, trimString } from '../../utils/isomorphic/stringUtils'; import { closestCrossShadow, isInsideScope, parentElementOrShadowHost } from './domUtils'; import type { InjectedScript } from './injectedScript'; import { getAriaRole, getElementAccessibleName, beginAriaCaches, endAriaCaches } from './roleUtils'; @@ -276,7 +276,7 @@ function buildTextCandidates(injectedScript: InjectedScript, element: Element, i } const fullText = normalizeWhiteSpace(elementText(injectedScript._evaluator._cacheText, element).full); - const text = fullText.substring(0, 80); + const text = trimString(fullText, 80); if (text) { const escaped = escapeForTextSelector(text, false); if (isTargetNode) { diff --git a/packages/playwright-core/src/utils/isomorphic/stringUtils.ts b/packages/playwright-core/src/utils/isomorphic/stringUtils.ts index ee2258641a..d51ecf8d41 100644 --- a/packages/playwright-core/src/utils/isomorphic/stringUtils.ts +++ b/packages/playwright-core/src/utils/isomorphic/stringUtils.ts @@ -103,3 +103,16 @@ export function escapeForAttributeSelector(value: string | RegExp, exact: boolea // so we escape them differently. return `"${value.replace(/\\/g, '\\\\').replace(/["]/g, '\\"')}"${exact ? 's' : 'i'}`; } + +export function trimString(input: string, cap: number, suffix: string = ''): string { + if (input.length <= cap) + return input; + const chars = [...input]; + if (chars.length > cap) + return chars.slice(0, cap - suffix.length).join('') + suffix; + return chars.join(''); +} + +export function trimStringWithEllipsis(input: string, cap: number): string { + return trimString(input, cap, '\u2026'); +} \ No newline at end of file diff --git a/tests/page/elementhandle-convenience.spec.ts b/tests/page/elementhandle-convenience.spec.ts index 21ef4c85bf..5020f34ea8 100644 --- a/tests/page/elementhandle-convenience.spec.ts +++ b/tests/page/elementhandle-convenience.spec.ts @@ -30,6 +30,13 @@ it('should have a nice preview', async ({ page, server }) => { expect(String(check)).toBe('JSHandle@'); }); +it('should have a nice preview for non-ascii attributes/children', async ({ page, server }) => { + await page.goto(server.EMPTY_PAGE); + await page.setContent(`
${'πŸ˜›'.repeat(100)}`); + const handle = await page.$('div'); + await expect.poll(() => String(handle)).toBe(`JSHandle@
πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›πŸ˜›β€¦
`); +}); + it('getAttribute should work', async ({ page, server }) => { await page.goto(`${server.PREFIX}/dom.html`); const handle = await page.$('#outer'); diff --git a/tests/page/page-wait-for-request.spec.ts b/tests/page/page-wait-for-request.spec.ts index 555ace31ad..3ba6e9304f 100644 --- a/tests/page/page-wait-for-request.spec.ts +++ b/tests/page/page-wait-for-request.spec.ts @@ -63,7 +63,7 @@ it('should respect default timeout', async ({ page, playwright }) => { it('should log the url', async ({ page }) => { const error = await page.waitForRequest('long-long-long-long-long-long-long-long-long-long-long-long-long-long.css', { timeout: 1000 }).catch(e => e); - expect(error.message).toContain('waiting for request "long-long-long-long-long-long-long-long-long-long-…"'); + expect(error.message).toContain('waiting for request "long-long-long-long-long-long-long-long-long-long…"'); }); it('should work with no timeout', async ({ page, server }) => {