fix(locators): allow identical frameLocators inside and/or/has (#23740)

So, the following will work:

```
page.frameLocator('iframe').locator('span').or(page.frameLoactor('iframe').locator('div'))
```

The following will not work, because frame locators are not exactly the
same:

```
page.frameLocator('#iframe1').locator('span').or(page.frameLoactor('#iframe2').locator('div'))
```

Also improve the error message to be more readable and include the
locator.

Fixes #23697.

packages/playwright-core/src/server/frameSelectors.ts

@@ -16,10 +16,11 @@
 
 import type { Frame } from './frames';
 import type * as types from './types';
-import { stringifySelector, type ParsedSelector, splitSelectorByFrame } from '../utils/isomorphic/selectorParser';
+import { stringifySelector, type ParsedSelector, splitSelectorByFrame, InvalidSelectorError, visitAllSelectorParts } from '../utils/isomorphic/selectorParser';
 import type { FrameExecutionContext, ElementHandle } from './dom';
 import type { JSHandle } from './javascript';
 import type { InjectedScript } from './injected/injectedScript';
+import { asLocator } from '../utils/isomorphic/locatorGenerators';
 
 export type SelectorInfo = {
   parsed: ParsedSelector,
@@ -111,6 +112,15 @@ export class FrameSelectors {
     let frame: Frame = this.frame;
     const frameChunks = splitSelectorByFrame(selector);
 
+    for (const chunk of frameChunks) {
+      visitAllSelectorParts(chunk, (part, nested) => {
+        if (nested && part.name === 'internal:control' && part.body === 'enter-frame') {
+          const locator = asLocator(this.frame._page.attribution.playwright.options.sdkLanguage, selector);
+          throw new InvalidSelectorError(`Frame locators are not allowed inside composite locators, while querying "${locator}"`);
+        }
+      });
+    }
+
     for (let i = 0; i < frameChunks.length - 1; ++i) {
       const info = this._parseSelector(frameChunks[i], options);
       const context = await frame._context(info.world);

packages/playwright-core/src/server/injected/injectedScript.ts

@@ -21,7 +21,7 @@ import { VueEngine } from './vueSelectorEngine';
 import { createRoleEngine } from './roleSelectorEngine';
 import { parseAttributeSelector } from '../../utils/isomorphic/selectorParser';
 import type { NestedSelectorBody, ParsedSelector, ParsedSelectorPart } from '../../utils/isomorphic/selectorParser';
-import { allEngineNames, parseSelector, stringifySelector } from '../../utils/isomorphic/selectorParser';
+import { visitAllSelectorParts, parseSelector, stringifySelector } from '../../utils/isomorphic/selectorParser';
 import { type TextMatcher, elementMatchesText, elementText, type ElementText } from './selectorUtils';
 import { SelectorEvaluatorImpl, sortInDOMOrder } from './selectorEvaluator';
 import { enclosingShadowRootOrDocument, isElementVisible, parentElementOrShadowHost } from './domUtils';
@@ -146,10 +146,10 @@ export class InjectedScript {
 
   parseSelector(selector: string): ParsedSelector {
     const result = parseSelector(selector);
-    for (const name of allEngineNames(result)) {
-      if (!this._engines.has(name))
-        throw this.createStacklessError(`Unknown engine "${name}" while parsing selector ${selector}`);
-    }
+    visitAllSelectorParts(result, part => {
+      if (!this._engines.has(part.name))
+        throw this.createStacklessError(`Unknown engine "${part.name}" while parsing selector ${selector}`);
+    });
     return result;
   }
 

packages/playwright-core/src/server/selectors.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { allEngineNames, InvalidSelectorError, type ParsedSelector, parseSelector, stringifySelector } from '../utils/isomorphic/selectorParser';
+import { visitAllSelectorParts, InvalidSelectorError, type ParsedSelector, parseSelector, stringifySelector } from '../utils/isomorphic/selectorParser';
 import { createGuid } from '../utils';
 
 export class Selectors {
@@ -73,7 +73,8 @@ export class Selectors {
   parseSelector(selector: string | ParsedSelector, strict: boolean) {
     const parsed = typeof selector === 'string' ? parseSelector(selector) : selector;
     let needsMainWorld = false;
-    for (const name of allEngineNames(parsed)) {
+    visitAllSelectorParts(parsed, part => {
+      const name = part.name;
       const custom = this._engines.get(name);
       if (!custom && !this._builtinEngines.has(name))
         throw new InvalidSelectorError(`Unknown engine "${name}" while parsing selector ${stringifySelector(parsed)}`);
@@ -81,7 +82,7 @@ export class Selectors {
         needsMainWorld = true;
       if (this._builtinEnginesInMainWorld.has(name))
         needsMainWorld = true;
-    }
+    });
     return {
       parsed,
       world: needsMainWorld ? 'main' as const : 'utility' as const,

packages/playwright-core/src/utils/isomorphic/selectorParser.ts

@@ -41,17 +41,19 @@ type ParsedSelectorStrings = {
 export const customCSSNames = new Set(['not', 'is', 'where', 'has', 'scope', 'light', 'visible', 'text', 'text-matches', 'text-is', 'has-text', 'above', 'below', 'right-of', 'left-of', 'near', 'nth-match']);
 
 export function parseSelector(selector: string): ParsedSelector {
-  const result = parseSelectorString(selector);
-  const parts: ParsedSelectorPart[] = result.parts.map(part => {
+  const parsedStrings = parseSelectorString(selector);
+  const parts: ParsedSelectorPart[] = [];
+  for (const part of parsedStrings.parts) {
     if (part.name === 'css' || part.name === 'css:light') {
       if (part.name === 'css:light')
         part.body = ':light(' + part.body + ')';
       const parsedCSS = parseCSS(part.body, customCSSNames);
-      return {
+      parts.push({
         name: 'css',
         body: parsedCSS.selector,
         source: part.body
-      };
+      });
+      continue;
     }
     if (kNestedSelectorNames.has(part.name)) {
       let innerSelector: string;
@@ -69,17 +71,21 @@ export function parseSelector(selector: string): ParsedSelector {
       } catch (e) {
         throw new InvalidSelectorError(`Malformed selector: ${part.name}=` + part.body);
       }
-      const result = { name: part.name, source: part.body, body: { parsed: parseSelector(innerSelector), distance } };
-      if (result.body.parsed.parts.some(part => part.name === 'internal:control' && part.body === 'enter-frame'))
-        throw new InvalidSelectorError(`Frames are not allowed inside "${part.name}" selectors`);
-      return result;
+      const nested = { name: part.name, source: part.body, body: { parsed: parseSelector(innerSelector), distance } };
+      const lastFrame = [...nested.body.parsed.parts].reverse().find(part => part.name === 'internal:control' && part.body === 'enter-frame');
+      const lastFrameIndex = lastFrame ? nested.body.parsed.parts.indexOf(lastFrame) : -1;
+      // Allow nested selectors to start with the same frame selector.
+      if (lastFrameIndex !== -1 && selectorPartsEqual(nested.body.parsed.parts.slice(0, lastFrameIndex + 1), parts.slice(0, lastFrameIndex + 1)))
+        nested.body.parsed.parts.splice(0, lastFrameIndex + 1);
+      parts.push(nested);
+      continue;
     }
-    return { ...part, source: part.body };
-  });
+    parts.push({ ...part, source: part.body });
+  }
   if (kNestedSelectorNames.has(parts[0].name))
     throw new InvalidSelectorError(`"${parts[0].name}" selector cannot be first`);
   return {
-    capture: result.capture,
+    capture: parsedStrings.capture,
     parts
   };
 }
@@ -113,6 +119,10 @@ export function splitSelectorByFrame(selectorText: string): ParsedSelector[] {
   return result;
 }
 
+function selectorPartsEqual(list1: ParsedSelectorPart[], list2: ParsedSelectorPart[]) {
+  return stringifySelector({ parts: list1 }) === stringifySelector({ parts: list2 });
+}
+
 export function stringifySelector(selector: string | ParsedSelector): string {
   if (typeof selector === 'string')
     return selector;
@@ -122,17 +132,15 @@ export function stringifySelector(selector: string | ParsedSelector): string {
   }).join(' >> ');
 }
 
-export function allEngineNames(selector: ParsedSelector): Set<string> {
-  const result = new Set<string>();
-  const visit = (selector: ParsedSelector) => {
+export function visitAllSelectorParts(selector: ParsedSelector, visitor: (part: ParsedSelectorPart, nested: boolean) => void) {
+  const visit = (selector: ParsedSelector, nested: boolean) => {
     for (const part of selector.parts) {
-      result.add(part.name);
+      visitor(part, nested);
       if (kNestedSelectorNames.has(part.name))
-        visit((part.body as NestedSelectorBody).parsed);
+        visit((part.body as NestedSelectorBody).parsed, true);
     }
   };
-  visit(selector);
-  return result;
+  visit(selector, false);
 }
 
 function parseSelectorString(selector: string): ParsedSelectorStrings {

tests/page/locator-query.spec.ts

@@ -189,6 +189,17 @@ it('should support locator.or', async ({ page }) => {
   await expect(page.locator('span').or(page.locator('article'))).toHaveText('world');
 });
 
+it('should allow some, but not all nested frameLocators', async ({ page }) => {
+  await page.setContent(`<iframe srcdoc="<span id=target>world</span>"></iframe><span>hello</span>`);
+  await expect(page.frameLocator('iframe').locator('span').or(page.frameLocator('iframe').locator('article'))).toHaveText('world');
+  await expect(page.frameLocator('iframe').locator('article').or(page.frameLocator('iframe').locator('span'))).toHaveText('world');
+  await expect(page.frameLocator('iframe').locator('span').and(page.frameLocator('iframe').locator('#target'))).toHaveText('world');
+  const error1 = await expect(page.frameLocator('iframe').locator('div').or(page.frameLocator('#iframe').locator('span'))).toHaveText('world').catch(e => e);
+  expect(error1.message).toContain(`Frame locators are not allowed inside composite locators, while querying "frameLocator('iframe').locator('div').or(frameLocator('#iframe').locator('span'))`);
+  const error2 = await expect(page.frameLocator('iframe').locator('div').and(page.frameLocator('#iframe').locator('span'))).toHaveText('world').catch(e => e);
+  expect(error2.message).toContain(`Frame locators are not allowed inside composite locators, while querying "frameLocator('iframe').locator('div').and(frameLocator('#iframe').locator('span'))`);
+});
+
 it('should enforce same frame for has/leftOf/rightOf/above/below/near', async ({ page, server }) => {
   await page.goto(server.PREFIX + '/frames/two-frames.html');
   const child = page.frames()[1];