Skip to content

fix(replay): Default ignore file inputs for dead clicks #14227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 19, 2024
Merged

fix(replay): Default ignore file inputs for dead clicks #14227

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab867d4
add default values to slow click ignore selector
chargome Nov 11, 2024
8c2c6da
add test
chargome Nov 11, 2024
737e7bc
remove default selectors
chargome Nov 14, 2024
1178c2e
Merge branch 'develop' into cg/ignore-deadclick-on-input-file
chargome Nov 19, 2024
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
41 changes: 41 additions & 0 deletions dev-packages/browser-integration-tests/suites/replay/slowClick/ignore/test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,44 @@ sentryTest('click is ignored on div', async ({ getLocalTestUrl, page }) => {
},
]);
});

sentryTest('click is ignored on input file by default', async ({ getLocalTestUrl, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}

const url = await getLocalTestUrl({ testDir: __dirname });

await Promise.all([waitForReplayRequest(page, 0), page.goto(url)]);

const [req1] = await Promise.all([
waitForReplayRequest(page, (event, res) => {
const { breadcrumbs } = getCustomRecordingEvents(res);

return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.click');
}),
page.locator('#inputFile').click(),
]);

const { breadcrumbs } = getCustomRecordingEvents(req1);

expect(breadcrumbs).toEqual([
{
category: 'ui.click',
data: {
node: {
attributes: {
id: 'inputFile',
},
id: expect.any(Number),
tagName: 'input',
textContent: '',
},
nodeId: expect.any(Number),
},
message: 'body > input#inputFile[type="file"]',
timestamp: expect.any(Number),
type: 'default',
},
]);
});
3 changes: 2 additions & 1 deletion dev-packages/browser-integration-tests/suites/replay/slowClick/template.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!DOCTYPE html>
<!doctype html>
<html>
<head>
<meta charset="utf-8" />
Expand Down Expand Up @@ -27,6 +27,7 @@
<input type="button" id="inputButton" value="Input button" />
<input type="submit" id="inputSubmit" value="Input submit" />
<input type="text" id="inputText" />
<input type="file" id="inputFile" />

<h1 id="h1">Heading</h1>

Expand Down
4 changes: 3 additions & 1 deletion packages/replay-internal/src/integration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const MEDIA_SELECTORS =

const DEFAULT_NETWORK_HEADERS = ['content-length', 'content-type', 'accept'];

const DEFAULT_SLOW_CLICK_IGNORE_SELECTORS = ['input[type="file"]'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm wait, do we not already do that? in handleClick.ts we have this code:

// If <input> tag, we only want to consider input[type='submit'] & input[type='button']
  if (node.tagName === 'INPUT' && !['submit', 'button'].includes(node.getAttribute('type') || '')) {
    return true;
  }

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah you are right, the test also passes without the change 🤔


let _initialized = false;

/**
Expand Down Expand Up @@ -102,7 +104,7 @@ export class Replay implements Integration {
mutationLimit = 10_000,

slowClickTimeout = 7_000,
slowClickIgnoreSelectors = [],
slowClickIgnoreSelectors = DEFAULT_SLOW_CLICK_IGNORE_SELECTORS,

networkDetailAllowUrls = [],
networkDetailDenyUrls = [],
Expand Down
Loading