Skip to content

Stabilize attachment Save test by settling scroll before opening the context menu #79

Open

Description

@dnlbui

The multiple-attachments E2E test can open an attachment's context menu and then immediately lose it to a pending scroll event generated while Playwright brings the attachment into view. The client correctly closes context menus on scrolling; the test needs to finish scrolling before opening the menu.

Failure observed in CI

  • Workflow run 37047911389, targeting https://liberdus.com/dev/.
  • Test checkout: 2fc3036c11900073e0616cf9ddcdf3cf233904e3.
  • Scenario: File Attachment Tests › should send and download multiple attachments, starting at playwright-tests/tests/attachments.e2e.test.js:460.
  • Chromium, four CI workers, slowMo: 1000, viewport 470 × 900.
  • The initial attempt failed; the first retry passed, including download and byte comparisons for all five attachments.

The primary failure was:

Timed out 5000ms waiting for expect(locator).toBeVisible()
Locator: #imageAttachmentContextMenu .context-menu-option[data-action="save"]
Expected: visible
Received: hidden

Playwright resolved a Save element with aria-disabled="false". Save was present and enabled; its parent menu was hidden.

The additional page.waitForEvent: Target page, context or browser has been closed error was secondary: the test had created a download waiter before opening the menu and then closed the recipient page during cleanup after the visibility assertion failed.

Root cause established by local reproduction

A temporary copy of the original scenario was run against the same dev site, preserving the attachment fixtures, interaction sequence, viewport, and one-second slow motion. Only diagnostic event logging and capture before cleanup were added; application source and network responses were unchanged.

The first four attachments downloaded successfully. On the fifth attachment:

  1. Playwright's click action logged scrolling into view if needed, then done scrolling.
  2. It clicked the correct .attachment-label.
  3. The menu opened.
  4. A message-container scroll event arrived 3.8 ms after the click, while the menu was still display: block.
  5. The client's scroll handler closed all context menus.
  6. The assertion found the enabled Save item inside a menu with display: none.

Relevant captured sequence, measured within the recipient page:

17885.3 ms  click .attachment-label for test-image-3-...-multi.png
17889.1 ms  messages-container scroll; scrollTop=301; menu display=block
afterward  #imageAttachmentContextMenu display=none
           Save exists; aria-disabled=false

No client scrollTop assignment occurred near the failed click; the last logged application assignment was about 9.6 seconds earlier. The trace therefore identifies a pending scroll from Playwright's click preparation interacting with the client's intended dismissal rule.

The original CI failure had no first-attempt trace, so its precise event order cannot be proven retrospectively. This local reproduction matches the same assertion and hidden-menu state without forcing a scroll or changing application behavior.

Exact code involved

Test code at lines 556–565:

// Starts waiting before it is known that the Save menu is open.
const downloadPromise = waitForNamedDownload(recipientPage, attachment.fileName);

// click() may scroll the attachment into view before dispatching the click.
await attachmentLink.click();
await expect(
  recipientPage.locator(
    '#imageAttachmentContextMenu .context-menu-option[data-action="save"]'
  )
).toBeVisible();

await recipientPage.click(
  '#imageAttachmentContextMenu .context-menu-option[data-action="save"]'
);
const download = await downloadPromise;

Client scroll handler:

handleMessagesContainerScroll() {
  this.closeAllContextMenus();
  // ...
}

This is intended client behavior and should remain intact.

Proposed resolution

  1. Explicitly bring the target attachment into view before attempting to open its menu.
  2. Wait, with a bounded helper, for the message container's scrolling/layout to settle and pending scroll events to be processed. Scrolling and clicking must be separate phases. Simply adding scrollIntoViewIfNeeded() and clicking immediately may retain the same event-order race.
  3. Click the attachment only after that preparation, then assert that the menu and Save option are visible.
  4. Start the named download waiter immediately before the Save click, and await both together. Ensure error paths do not leave an unobserved download promise.
  5. Preserve the existing filename, file-size, and byte-for-byte assertions for every attachment.
  6. Capture the failed recipient page before cleanup, using the shared diagnostics work in Improve lookup and account-setup diagnostics and retain first-failure E2E evidence #78.

Illustrative target flow; waitForMessageScrollSettled is a helper to implement and validate, not an existing API or a verified patch:

await attachmentLink.scrollIntoViewIfNeeded();
await waitForMessageScrollSettled(
  recipientPage.locator('#chatModal .messages-container')
);

await attachmentLink.click();

const saveOption = recipientPage.locator(
  '#imageAttachmentContextMenu .context-menu-option[data-action="save"]'
);
await expect(saveOption).toBeVisible();

const [download] = await Promise.all([
  waitForNamedDownload(recipientPage, attachment.fileName),
  saveOption.click(),
]);

Prefer observing settled scroll position/layout across rendered frames with a deadline over adding a large unconditional sleep. Verify the helper in supported browsers. Do not force clicks, bypass the menu with JavaScript, remove the client's scroll listener, or increase the visibility timeout: waiting longer does not reopen a closed menu.

Verification and acceptance criteria

  • All five attachments can be saved and their bytes verified, including an attachment initially outside the visible portion of the message container.
  • Menu-opening clicks occur after scroll settlement, and no queued preparation scroll closes the newly opened menu.
  • The scenario passes repeated focused runs with retries disabled; verify the originally failing Chromium configuration first.
  • Intentional scrolling/outside actions still dismiss the menu in the client.
  • The original assertion remains the primary reported failure when an error is deliberately introduced; no dangling download waiter adds a misleading page-closed error.
  • No production client behavior changes are needed.

Suggested focused command:

cd playwright-tests
CI=true npx playwright test tests/attachments.e2e.test.js \
  --project=chromium \
  --grep "should send and download multiple attachments" \
  --workers=1 --retries=0 --repeat-each=3 --trace=on

Invalid image/PDF fixture generation also produced broken preview tabs during investigation, but it is a separate fixture-quality problem and was not the cause identified for this reproduced menu closure. Shared first-failure diagnostics are tracked in #78. No fix has been applied yet.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions