Skip to content

Commit a3f167f

Browse files
ptrthomasclaude
andcommitted
perf(driver): ask the resolver for the candidate list once, not once per index
Deriving a locator asked "which index am I?" by calling resolve() for 1, 2, 3 … 25 — and every one of those re-scans the document (getVisibleText + isVisible + hasMatchingDescendant over the whole candidate set). The loop only ends early when the element WINS an index, so an element whose text repeats — a column header standing in six list views on an enterprise screen — pays all 25 to learn it has no wildcard identity at all. Measured on a live PolicyCenter page (2030 nodes, ~220 candidates): ~400ms for one such element, and ~25s for the snapshot the agent layer takes before every state-changing action, with CDP Runtime.evaluate timeouts on top. - resolveAll(tag, text, contains) returns exactly the candidates resolve() would consider, in resolve()'s own order: same predicate, same ranker seam, same shadow-DOM fallback. resolveAll(...)[i - 1] === resolve(..., i, ...). - the predicate itself moves into _matchList, so the ranked path and the new one share one implementation of "what matches" (the unranked path keeps its own short-circuiting copy — it can stop at the index-th match). - the text test now runs BEFORE isVisible in every path. isVisible ends in getBoundingClientRect + elementFromPoint — a layout flush and a hit test — and running it first spent both on every node on the page, nearly all of which never matched. Same result set, strictly less work. Four e2e cases pin the contract, the equivalence one asserted inside the page so it compares element identity rather than ids: index-for-index agreement with resolve() (and nothing one past the list), non-candidates reported absent rather than guessed, the ranked order honoured, and the shadow-DOM sweep. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent c4650e9 commit a3f167f

2 files changed

Lines changed: 121 additions & 12 deletions

File tree

karate-core/src/main/resources/io/karatelabs/driver/driver.js

Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,16 @@
282282
var descendants = el.querySelectorAll(selector);
283283
for (var i = 0; i < descendants.length; i++) {
284284
var desc = descendants[i];
285-
if (!this.isVisible(desc)) continue;
285+
// Text BEFORE visibility, deliberately: isVisible() ends in getBoundingClientRect +
286+
// elementFromPoint — a layout flush and a hit test — while getVisibleText() only reads
287+
// styles. Same result set, but the expensive half now runs only for elements whose text
288+
// actually matches, instead of for every node on the page. (See _matchList.)
286289
var descText = this.getVisibleText(desc);
287290
if (!descText) continue;
288291
var matches = contains
289292
? descText.indexOf(text) !== -1
290293
: descText === text;
291-
if (matches) return true;
294+
if (matches && this.isVisible(desc)) return true;
292295
}
293296
return false;
294297
};
@@ -340,18 +343,19 @@
340343
var count = 0;
341344
for (var i = 0; i < candidates.length; i++) {
342345
var el = candidates[i];
343-
if (!this.isVisible(el)) continue;
344346
if (text === '') {
347+
if (!this.isVisible(el)) continue;
345348
count++;
346349
if (count === index) return el;
347350
continue;
348351
}
352+
// text first, then the layout-forcing visibility test — see _matchList
349353
var elText = this.getVisibleText(el);
350354
if (!elText) continue;
351355
var matches = contains
352356
? elText.indexOf(text) !== -1
353357
: elText === text;
354-
if (matches) {
358+
if (matches && this.isVisible(el)) {
355359
if (this.hasMatchingDescendant(el, text, contains, selector)) continue;
356360
count++;
357361
if (count === index) return el;
@@ -361,29 +365,74 @@
361365
};
362366

363367
/**
364-
* Ranked resolution path (active only when a ranker is registered).
365-
* Collects ALL matches with the same predicate the default path applies,
366-
* hands them to the ranker, then applies the same index selection to the
367-
* ranked list \u2014 so an identity ranker reproduces default behavior exactly.
368+
* The predicate itself, as a list: every candidate satisfying (tag, text),
369+
* in DOM order. The unranked path above keeps its own short-circuiting copy
370+
* (it can stop at the index-th match); everything that needs the WHOLE list
371+
* comes through here, so there is one implementation of "what matches".
368372
*/
369-
kjs._resolveRanked = function(candidates, tag, text, index, contains, selector) {
373+
kjs._matchList = function(candidates, tag, text, contains, selector) {
370374
var matches = [];
371375
for (var i = 0; i < candidates.length; i++) {
372376
var el = candidates[i];
373-
if (!this.isVisible(el)) continue;
374377
if (text === '') {
375-
matches.push(el);
378+
if (this.isVisible(el)) matches.push(el);
376379
continue;
377380
}
381+
// Cheap test first. isVisible() forces layout (getBoundingClientRect) and hit-tests
382+
// (elementFromPoint) — on a `*` resolve over an enterprise page that is thousands of
383+
// layout flushes, and all but a handful are spent on elements whose text never matched.
384+
// getVisibleText() reads styles only. The set is identical; the order is not.
378385
var elText = this.getVisibleText(el);
379386
if (!elText) continue;
380387
var ok = contains
381388
? elText.indexOf(text) !== -1
382389
: elText === text;
383-
if (ok && !this.hasMatchingDescendant(el, text, contains, selector)) {
390+
if (ok && this.isVisible(el) && !this.hasMatchingDescendant(el, text, contains, selector)) {
384391
matches.push(el);
385392
}
386393
}
394+
return matches;
395+
};
396+
397+
/**
398+
* Every candidate resolve() would consider for (tag, text), in resolve()'s
399+
* own order — same visibility + leaf-text predicate, same ranker seam, same
400+
* shadow-DOM fallback (the deep sweep runs when the light DOM matched none).
401+
*
402+
* This exists for the caller that needs an element's INDEX rather than the
403+
* element at an index: without it, "which index am I?" is answered by calling
404+
* resolve() for 1, 2, 3 … n, and every one of those re-scans the document.
405+
* On a page whose text repeats — an enterprise screen with the same column
406+
* header in six list views — that walk dominates the caller's whole run.
407+
* `resolveAll(tag, text, contains)[i - 1] === resolve(tag, text, i, contains)`.
408+
*/
409+
kjs.resolveAll = function(tag, text, contains) {
410+
var selector = this.getSelector(tag);
411+
text = ('' + (text || '')).replace(/\s+/g, ' ').trim();
412+
var matches = this._matchList(document.querySelectorAll(selector), tag, text, contains, selector);
413+
if (!matches.length && this.hasShadowDOM()) {
414+
matches = this._matchList(this.querySelectorAllDeep(selector), tag, text, contains, selector);
415+
}
416+
if (!this._resolveRanker) return matches;
417+
try {
418+
var ranked = this._resolveRanker(matches.slice(), {
419+
tag: tag, text: text, index: 1, contains: contains, selector: selector
420+
});
421+
if (Array.isArray(ranked)) return ranked;
422+
} catch (e) {
423+
this.log('resolve ranker failed, using default order', {error: '' + e});
424+
}
425+
return matches;
426+
};
427+
428+
/**
429+
* Ranked resolution path (active only when a ranker is registered).
430+
* Collects ALL matches with the same predicate the default path applies,
431+
* hands them to the ranker, then applies the same index selection to the
432+
* ranked list \u2014 so an identity ranker reproduces default behavior exactly.
433+
*/
434+
kjs._resolveRanked = function(candidates, tag, text, index, contains, selector) {
435+
var matches = this._matchList(candidates, tag, text, contains, selector);
387436
var ranked = matches;
388437
try {
389438
var result = this._resolveRanker(matches.slice(), {

karate-core/src/test/java/io/karatelabs/driver/e2e/LocatorsE2eTest.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,4 +567,64 @@ void testResolveRankerMayAddCandidatesTheDefaultFilterExcluded() {
567567
assertEquals("hidden-menu", driver.attribute("{div}Save", "id"));
568568
}
569569

570+
// ========== resolveAll (the candidate list behind resolve) ==========
571+
// Asking "which index am I?" used to mean calling resolve() for 1, 2, 3 … n, and each of
572+
// those re-scans the document — on an enterprise page whose text repeats, that walk cost
573+
// hundreds of ms PER ELEMENT and dominated the caller's run. resolveAll answers it in one
574+
// scan, and is only worth having if it is EXACTLY what resolve() would have said.
575+
576+
@Test
577+
void testResolveAllIsIndexByIndexWhatResolveReturns() {
578+
driver.setUrl("data:text/html,"
579+
+ "<b id='one'>Status</b><i>skip</i><b id='two'>Status</b>"
580+
+ "<b id='three'>Status</b><b id='other'>Elsewhere</b>");
581+
// the equivalence, asserted in the page so it compares element IDENTITY, not ids
582+
Object verdict = driver.script("(function(){"
583+
+ " var all = window.__kjs.resolveAll('b', 'Status', false);"
584+
+ " for (var i = 0; i < all.length; i++) {"
585+
+ " if (all[i] !== window.__kjs.resolve('b', 'Status', i + 1, false)) return 'differs at ' + (i + 1);"
586+
+ " }"
587+
+ " if (window.__kjs.resolve('b', 'Status', all.length + 1, false)) return 'resolve saw one past the list';"
588+
+ " return 'ok:' + all.length; })()");
589+
assertEquals("ok:3", String.valueOf(verdict));
590+
}
591+
592+
@Test
593+
void testResolveAllReportsNonCandidatesAsAbsentRatherThanGuessing() {
594+
driver.setUrl("data:text/html,<b id='hit'>Status</b><b id='miss'>Other</b>");
595+
Object verdict = driver.script("(function(){"
596+
+ " var all = window.__kjs.resolveAll('b', 'Status', false);"
597+
+ " return [all.length,"
598+
+ " all.indexOf(document.getElementById('hit')),"
599+
+ " all.indexOf(document.getElementById('miss')),"
600+
+ " window.__kjs.resolveAll('b', 'NothingHere', false).length].join(','); })()");
601+
assertEquals("1,0,-1,0", String.valueOf(verdict));
602+
}
603+
604+
@Test
605+
void testResolveAllHonoursTheRankerSoItStillAgreesWithResolve() {
606+
driver.setUrl("data:text/html,<b id='one'>X</b><b id='two'>X</b><b id='three'>X</b>");
607+
driver.script("window.__kjs.setResolveRanker(function(m){ m.push(m.shift()); return m; })");
608+
Object ranked = driver.script("(function(){"
609+
+ " var all = window.__kjs.resolveAll('b', 'X', false);"
610+
+ " var ids = []; for (var i = 0; i < all.length; i++) ids.push(all[i].id);"
611+
+ " return ids.join(','); })()");
612+
assertEquals("two,three,one", String.valueOf(ranked));
613+
// …and index-for-index that is still what resolve() hands back under the same ranker
614+
assertEquals("two", driver.attribute("{b:1}X", "id"));
615+
assertEquals("one", driver.attribute("{b:3}X", "id"));
616+
}
617+
618+
@Test
619+
void testResolveAllFallsBackToTheDeepSweepLikeResolveDoes() {
620+
driver.setUrl("data:text/html,<div id='host'></div>");
621+
driver.script("(function(){ var h = document.getElementById('host').attachShadow({mode:'open'});"
622+
+ " h.innerHTML = \"<b id='inner'>Shadowed</b>\"; })()");
623+
Object verdict = driver.script("(function(){"
624+
+ " var all = window.__kjs.resolveAll('b', 'Shadowed', false);"
625+
+ " if (all.length !== 1) return 'list length ' + all.length;"
626+
+ " return all[0] === window.__kjs.resolve('b', 'Shadowed', 1, false) ? 'same' : 'different'; })()");
627+
assertEquals("same", String.valueOf(verdict));
628+
}
629+
570630
}

0 commit comments

Comments
 (0)