mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 14:16:28 +03:00
Fix: skipScan must cover the visual contrast stage too
Bugbot on PR #665: the skipScan guard emptied only the analytic collectBrowserFindings pass, and scan()'s detached visual-contrast stage then repopulated an ignoreFiles-waived page with contrast markers and a second non-zero results post. Hoist the guard into skipScanActive() and honor it in scan() and the async collector; regenerate the browser bundle. Adds a browser-backed regression test that reproduces the leak (second results post carrying low-contrast findings) and pins the zero contract; drops a tautological assert flagged in review. AI-assisted change: implemented with Claude Code under maintainer direction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
f031eb0b9f
commit
281f30e5cc
@@ -1472,13 +1472,17 @@ if (IS_BROWSER) {
|
||||
return findings;
|
||||
}
|
||||
|
||||
// A page matched by detector.ignoreFiles is waived wholesale: every scan
|
||||
// stage answers empty so the badge and toast read zero. Mirrors
|
||||
// shouldIgnoreDetectionFile in cli/lib/impeccable-config.mjs; the live
|
||||
// overlay resolves the globs per page (live-browser-ignores.js) and
|
||||
// forwards the verdict as config.skipScan.
|
||||
function skipScanActive() {
|
||||
return EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true;
|
||||
}
|
||||
|
||||
function collectBrowserFindings() {
|
||||
// A page matched by detector.ignoreFiles is waived wholesale: answer the
|
||||
// scan with the empty shape so the badge and toast read zero. Mirrors
|
||||
// shouldIgnoreDetectionFile in cli/lib/impeccable-config.mjs; the live
|
||||
// overlay resolves the globs per page (live-browser-ignores.js) and
|
||||
// forwards the verdict as config.skipScan.
|
||||
if (EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true) {
|
||||
if (skipScanActive()) {
|
||||
return { groupMap: new Map(), allFindings: [], pageLevelFindings: [] };
|
||||
}
|
||||
const groupMap = new Map();
|
||||
@@ -2013,6 +2017,12 @@ if (IS_BROWSER) {
|
||||
|
||||
async function collectBrowserFindingsAsync(options = {}, runtime = {}) {
|
||||
const collected = collectBrowserFindings();
|
||||
// The visual pass walks the DOM on its own; on a skipScan page it would
|
||||
// repopulate the emptied scan, so it is skipped with everything else.
|
||||
if (skipScanActive()) {
|
||||
lastVisualContrastAnalyses = [];
|
||||
return { ...collected, allFindings: [], visualContrastAnalyses: [] };
|
||||
}
|
||||
await addVisualContrastFindings(collected.groupMap, options, runtime);
|
||||
return {
|
||||
...collected,
|
||||
@@ -2066,7 +2076,7 @@ if (IS_BROWSER) {
|
||||
const generation = scanGeneration;
|
||||
const collected = collectBrowserFindings();
|
||||
const allFindings = renderBrowserFindings(collected, options);
|
||||
if (shouldRunVisualContrast(options)) {
|
||||
if (!skipScanActive() && shouldRunVisualContrast(options)) {
|
||||
addVisualContrastFindings(collected.groupMap, options, { decorate: true, generation })
|
||||
.then(() => {
|
||||
if (generation === scanGeneration) postSerializedFindings(collected.groupMap, options);
|
||||
|
||||
@@ -8131,13 +8131,17 @@ if (IS_BROWSER) {
|
||||
return findings;
|
||||
}
|
||||
|
||||
// A page matched by detector.ignoreFiles is waived wholesale: every scan
|
||||
// stage answers empty so the badge and toast read zero. Mirrors
|
||||
// shouldIgnoreDetectionFile in cli/lib/impeccable-config.mjs; the live
|
||||
// overlay resolves the globs per page (live-browser-ignores.js) and
|
||||
// forwards the verdict as config.skipScan.
|
||||
function skipScanActive() {
|
||||
return EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true;
|
||||
}
|
||||
|
||||
function collectBrowserFindings() {
|
||||
// A page matched by detector.ignoreFiles is waived wholesale: answer the
|
||||
// scan with the empty shape so the badge and toast read zero. Mirrors
|
||||
// shouldIgnoreDetectionFile in cli/lib/impeccable-config.mjs; the live
|
||||
// overlay resolves the globs per page (live-browser-ignores.js) and
|
||||
// forwards the verdict as config.skipScan.
|
||||
if (EXTENSION_MODE && window.__IMPECCABLE_CONFIG__?.skipScan === true) {
|
||||
if (skipScanActive()) {
|
||||
return { groupMap: new Map(), allFindings: [], pageLevelFindings: [] };
|
||||
}
|
||||
const groupMap = new Map();
|
||||
@@ -8672,6 +8676,12 @@ if (IS_BROWSER) {
|
||||
|
||||
async function collectBrowserFindingsAsync(options = {}, runtime = {}) {
|
||||
const collected = collectBrowserFindings();
|
||||
// The visual pass walks the DOM on its own; on a skipScan page it would
|
||||
// repopulate the emptied scan, so it is skipped with everything else.
|
||||
if (skipScanActive()) {
|
||||
lastVisualContrastAnalyses = [];
|
||||
return { ...collected, allFindings: [], visualContrastAnalyses: [] };
|
||||
}
|
||||
await addVisualContrastFindings(collected.groupMap, options, runtime);
|
||||
return {
|
||||
...collected,
|
||||
@@ -8725,7 +8735,7 @@ if (IS_BROWSER) {
|
||||
const generation = scanGeneration;
|
||||
const collected = collectBrowserFindings();
|
||||
const allFindings = renderBrowserFindings(collected, options);
|
||||
if (shouldRunVisualContrast(options)) {
|
||||
if (!skipScanActive() && shouldRunVisualContrast(options)) {
|
||||
addVisualContrastFindings(collected.groupMap, options, { decorate: true, generation })
|
||||
.then(() => {
|
||||
if (generation === scanGeneration) postSerializedFindings(collected.groupMap, options);
|
||||
|
||||
@@ -1109,7 +1109,6 @@ describe('detectUrl — browser-only fixtures', () => {
|
||||
const hexWaiver = rgbToHex(rgbColor);
|
||||
const colorFiltered = await scan('scan-dv-3', [{ rule: 'design-system-color', value: hexWaiver }]);
|
||||
const waivedColorCount = unfiltered.colorValues.filter(value => value === rgbColor).length;
|
||||
assert.ok(waivedColorCount > 0);
|
||||
assert.equal(
|
||||
colorFiltered.colors,
|
||||
unfiltered.colors - waivedColorCount,
|
||||
@@ -1131,6 +1130,84 @@ describe('detectUrl — browser-only fixtures', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('extension scan: skipScan suppresses the visual contrast stage too', async () => {
|
||||
const puppeteer = await import('puppeteer');
|
||||
const browser = await puppeteer.default.launch({
|
||||
headless: true,
|
||||
args: process.env.CI ? ['--no-sandbox', '--disable-setuid-sandbox'] : [],
|
||||
});
|
||||
try {
|
||||
const page = await browser.newPage();
|
||||
// Keep failing visual-contrast cards inside the no-scroll viewport.
|
||||
await page.setViewport({ width: 1280, height: 1000 });
|
||||
await page.goto(`${baseUrl}/fixtures/antipatterns/visual-contrast.html`, { waitUntil: 'load' });
|
||||
const browserScript = fs.readFileSync(path.join(ROOT, 'cli/engine/detect-antipatterns-browser.js'), 'utf-8');
|
||||
await page.evaluate(() => {
|
||||
document.documentElement.dataset.impeccableExtension = 'true';
|
||||
window.__impeccableMessages = [];
|
||||
window.addEventListener('message', event => {
|
||||
if (event.source !== window || !event.data?.source?.startsWith('impeccable-')) return;
|
||||
window.__impeccableMessages.push(event.data);
|
||||
});
|
||||
});
|
||||
await page.evaluate(browserScript);
|
||||
const resultsFor = (scanId) => page.evaluate((id) => (
|
||||
(window.__impeccableMessages || [])
|
||||
.filter(m => m.source === 'impeccable-results' && m.scanId === id)
|
||||
.map(m => ({
|
||||
count: m.count,
|
||||
types: (m.findings || []).flatMap(g => (g.findings || []).map(f => f.type || f.id)),
|
||||
}))
|
||||
), scanId);
|
||||
|
||||
// Control: the visual pass runs after the analytic scan and re-posts
|
||||
// results carrying its low-contrast findings. This is exactly what an
|
||||
// ignoreFiles-waived page must not do.
|
||||
await page.evaluate(() => {
|
||||
window.postMessage({
|
||||
source: 'impeccable-command',
|
||||
action: 'scan',
|
||||
config: { scanId: 'vc-skip-1', visualContrast: true, visualContrastMaxCandidates: 20 },
|
||||
}, '*');
|
||||
});
|
||||
const controlDeadline = Date.now() + 8000;
|
||||
let control = [];
|
||||
while (Date.now() < controlDeadline) {
|
||||
control = await resultsFor('vc-skip-1');
|
||||
if (control.some(r => r.types.includes('low-contrast'))) break;
|
||||
await new Promise(resolve => setTimeout(resolve, 100));
|
||||
}
|
||||
assert.ok(
|
||||
control.some(r => r.types.includes('low-contrast')),
|
||||
`expected the control scan's visual pass to report low-contrast, got: ${JSON.stringify(control)}`,
|
||||
);
|
||||
|
||||
// skipScan: a page waived wholesale by detector.ignoreFiles must stay
|
||||
// at zero through the async visual stage as well: no results post with
|
||||
// findings, no markers.
|
||||
await page.evaluate(() => {
|
||||
window.postMessage({
|
||||
source: 'impeccable-command',
|
||||
action: 'scan',
|
||||
config: { scanId: 'vc-skip-2', visualContrast: true, visualContrastMaxCandidates: 20, skipScan: true },
|
||||
}, '*');
|
||||
});
|
||||
await new Promise(resolve => setTimeout(resolve, 2500));
|
||||
const skipped = await resultsFor('vc-skip-2');
|
||||
assert.ok(skipped.length >= 1, `expected the skipScan scan to post results, got: ${JSON.stringify(skipped)}`);
|
||||
assert.ok(
|
||||
skipped.every(r => r.count === 0 && r.types.length === 0),
|
||||
`expected every skipScan results post to stay empty, got: ${JSON.stringify(skipped)}`,
|
||||
);
|
||||
const overlays = await page.evaluate(() =>
|
||||
document.querySelectorAll('.impeccable-overlay, .impeccable-label').length);
|
||||
assert.equal(overlays, 0, `expected no markers on a skipScan page, got ${overlays}`);
|
||||
await page.close();
|
||||
} finally {
|
||||
await browser.close().catch(() => {});
|
||||
}
|
||||
});
|
||||
|
||||
it('browser API: impeccableDetect is pure, impeccableScan decorates', async () => {
|
||||
const puppeteer = await import('puppeteer');
|
||||
const browser = await puppeteer.default.launch({
|
||||
|
||||
Reference in New Issue
Block a user