Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions packages/@d-zero/anatomist/src/capture-layout-tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,58 @@ describe('captureLayoutTree', () => {
expect(result.root?.children[0]?.tagName).toBe('P');
});

it('prefers a higher-priority selector even when it matches later in DOM order', () => {
const doc = createDocument(
'<body><div class="main" data-rect="0,0,1,1">Wrong</div>' +
'<main id="page" data-rect="0,0,800,600">Right</main></body>',
);

const result = captureLayoutTree(null, SELECTORS, FALLBACK_SELECTORS, 10, doc);

expect(result.mainSelector).toBe('main#page');
});

it('prefers a nested higher-priority match over its lower-priority ancestor wrapper', () => {
const doc = createDocument(
'<body><div class="main" data-rect="0,0,800,600">' +
'<p data-rect="0,0,100,20">breadcrumb</p>' +
'<main id="page" data-rect="0,20,400,580">Real main</main>' +
'<div id="aside" data-rect="400,20,400,580">Sidebar</div>' +
'</div></body>',
);

const result = captureLayoutTree(null, SELECTORS, FALLBACK_SELECTORS, 10, doc);

expect(result.mainSelector).toBe('main#page');
});

it('skips an invalid selector in the priority list and keeps trying the rest', () => {
const doc = createDocument(
'<body><main id="page" data-rect="0,0,800,600">Right</main></body>',
);

const result = captureLayoutTree(
null,
['[[[invalid', ...SELECTORS],
FALLBACK_SELECTORS,
10,
doc,
);

expect(result.mainSelector).toBe('main#page');
});

it('prefers the earlier-listed selector when two selectors both match different elements', () => {
const doc = createDocument(
'<body><div class="main" data-rect="0,0,1,1">Wrong</div>' +
'<div id="main" data-rect="0,0,800,600">Right</div></body>',
);

const result = captureLayoutTree(null, SELECTORS, FALLBACK_SELECTORS, 10, doc);

expect(result.mainSelector).toBe('div#main');
});

it('falls back to the fallback selector list when the priority list has no match', () => {
const doc = createDocument(
'<body><div id="primaryMain" data-rect="0,0,800,600">Fallback</div></body>',
Expand All @@ -70,6 +122,26 @@ describe('captureLayoutTree', () => {
expect(result.mainSelector).toBe('div#primaryMain');
});

it('still returns DOM-order-first match among fallback selectors (fallback list is not priority-ordered per element)', () => {
const doc = createDocument(
'<body><div id="wrapperMain" data-rect="0,0,800,600">' +
'<p data-rect="0,0,100,20">breadcrumb</p>' +
'<div id="innerMain" data-rect="0,20,800,580">Inner</div>' +
'</div></body>',
);

// Both #wrapperMain and #innerMain match the same fallback selector
// (`[id*="main" i]`), so — unlike the priority list above — there is
// no higher/lower priority to arbitrate between them. `querySelector`
// returns whichever matches first in document order, which is the
// outer wrapper here. This is accepted, existing behavior: the
// priority-list fix only orders *between* selectors in the array,
// not among multiple elements matching the *same* selector.
const result = captureLayoutTree(null, SELECTORS, FALLBACK_SELECTORS, 10, doc);

expect(result.mainSelector).toBe('div#wrapperMain');
});

it('tries the explicit mainContentSelector before the priority list', () => {
const doc = createDocument(
'<body><main data-rect="0,0,1,1">Wrong</main><section id="custom" data-rect="0,0,800,600">Right</section></body>',
Expand Down
39 changes: 34 additions & 5 deletions packages/@d-zero/anatomist/src/capture-layout-tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@
* can match a different element than the one actually found, so this
* module resolves the element itself directly instead of round-tripping
* through that string.
*
* WHY the priority-list resolution logic in `resolveMainElement` is
* duplicated rather than shared: same closure-free constraint as above.
* `extractMainContentsFromDocument` (`@d-zero/beholder/get-main-contents.ts`)
* independently re-implements the identical "try each selector in array
* order, first match wins" strategy for the same reason. The two copies
* have drifted out of sync before (one queried the array with a single
* `querySelector(selectors.join(','))` — which resolves by DOM document
* order, not array priority — while the other still tried selectors one at
* a time); `capture-layout-tree.spec.ts` and `get-main-contents.spec.ts`
* both carry matching regression cases (searchable by the test name
* "prefers a higher-priority selector...") specifically so that fixing one
* side's resolution logic without the other shows up as a spec gap, not a
* silent divergence.
* @module
*/

Expand Down Expand Up @@ -117,12 +131,27 @@ export function captureLayoutTree(
}
}

// Tried one at a time, in priority order, rather than joined into a
// single `querySelector(selectors.join(','))` call: a CSS group
// selector matches whichever selector is first in *document order*,
// not whichever is first in this array — an ancestor wrapper matching
// a low-priority selector (e.g. `#contents`) would win over a
// descendant matching a higher-priority one (e.g. `#main`) just
// because it appears earlier in the DOM. Querying one selector at a
// time makes this array's order the actual priority.
let element: Element | null = null;
try {
element = doc.querySelector(selectors.join(','));
} catch {
// The built-in selector list is a fixed, known-valid constant, so
// this should be unreachable — but fail closed rather than throw.
for (const sel of selectors) {
try {
element = doc.querySelector(sel);
} catch {
// The built-in selector list is a fixed, known-valid constant,
// so this should be unreachable — but fail closed rather than
// aborting the whole priority list over one bad entry.
continue;
}
if (element) {
break;
}
}

if (element) {
Expand Down
49 changes: 47 additions & 2 deletions packages/@d-zero/beholder/src/get-main-contents.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,18 +203,63 @@ describe('extractMainContentsFromDocument', () => {
expect(result.wordCount).toBe(15);
});

it('returns first matching element in DOM order when multiple Phase-1 selectors match', () => {
it('prefers a higher-priority selector even when it matches later in DOM order', () => {
const result = extract(`
<html><body>
<main>Semantic main</main>
<div id="content">ID content</div>
<main>Semantic main</main>
</body></html>
`);

expect(result.main?.nodeName).toBe('MAIN');
expect(result.wordCount).toBe(12);
});

it('prefers a nested higher-priority match over its lower-priority ancestor wrapper', () => {
const result = extract(`
<html><body>
<div id="contents">
<p>breadcrumb</p>
<div id="main">Real main</div>
<div id="aside">Sidebar</div>
</div>
</body></html>
`);

expect(result.main?.id).toBe('main');
});

it('prefers the earlier-listed selector when two selectors both match different elements', () => {
const result = extract(`
<html><body>
<div class="main">Wrong</div>
<div id="main">Right</div>
</body></html>
`);

expect(result.main?.id).toBe('main');
});

it('still returns DOM-order-first match among fallback selectors (fallback list is not priority-ordered per element)', () => {
// Both #wrapperMain and #innerMain match the same fallback selector
// (`[id*="main" i]`), so — unlike the priority list above — there is
// no higher/lower priority to arbitrate between them. `querySelector`
// returns whichever matches first in document order, which is the
// outer wrapper here. This is accepted, existing behavior: the
// priority-list fix only orders *between* selectors in the array,
// not among multiple elements matching the *same* selector.
const result = extract(`
<html><body>
<div id="wrapperMain">
<p>breadcrumb</p>
<div id="innerMain">Inner</div>
</div>
</body></html>
`);

expect(result.main?.id).toBe('wrapperMain');
});

it('uses Phase-2 class*=main when Phase-1 finds nothing', () => {
const result = extract(
'<html><body><div class="page-main-area">Phase two</div></body></html>',
Expand Down
40 changes: 33 additions & 7 deletions packages/@d-zero/beholder/src/get-main-contents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@
*
* WHY no `@medv/finder`: selector strings are diagnostic only; a tag+id+class // cspell:disable-line
* path is enough and avoids a Node-only dependency inside the page realm.
*
* WHY the priority-list resolution logic below is duplicated rather than
* shared: same closure-free constraint as above. `@d-zero/anatomist`'s
* `capture-layout-tree.ts` independently re-implements the identical "try
* each selector in array order, first match wins" strategy for the same
* reason. The two copies have drifted out of sync before (one queried the
* array with a single `querySelector(selectors.join(','))` — which resolves
* by DOM document order, not array priority — while the other still tried
* selectors one at a time); this file's spec and
* `anatomist/capture-layout-tree.spec.ts` both carry matching regression
* cases (searchable by the test name "prefers a higher-priority
* selector...") specifically so that fixing one side's resolution logic
* without the other shows up as a spec gap, not a silent divergence.
* @module
*/

Expand Down Expand Up @@ -118,14 +131,27 @@ export function extractMainContentsFromDocument(
selectors.unshift(mainContentSelector);
}

// Tried one at a time, in priority order, rather than joined into a
// single `querySelector(selectors.join(','))` call: a CSS group selector
// matches whichever selector is first in *document order*, not whichever
// is first in this array — an ancestor wrapper matching a low-priority
// selector (e.g. `#contents`) would win over a descendant matching a
// higher-priority one (e.g. `#main`) just because it appears earlier in
// the DOM. Querying one selector at a time makes this array's order the
// actual priority.
let $main: Element | null = null;
try {
$main = doc.querySelector(selectors.join(','));
} catch {
// Invalid custom selector: retry without it so built-in selectors still run.
if (mainContentSelector) {
selectors.shift();
$main = doc.querySelector(selectors.join(','));
for (const sel of selectors) {
try {
$main = doc.querySelector(sel);
} catch {
// Invalid selector — only reachable for the caller-supplied
// mainContentSelector unshifted above, since the built-in list is
// a fixed, known-valid constant. Skip it and keep trying the
// remaining selectors in priority order.
continue;
}
if ($main) {
break;
}
}

Expand Down
Loading