Skip to content

return elements in document order from QueryAll #5307

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

return elements in document order from QueryAll #5307

wants to merge 1 commit into from
+89 −10

Conversation

shota3506
Copy link

What?

Fix #4367

This PR ensures that page.$$() and element.$$() (querySelectorAll) return elements in document order, matching the Web API specification behavior.

Why?

Previously, elements were returned in random order because Go's map iteration order is not guaranteed. The fix sorts elements by their numeric index before returning them.

While this adds a sorting cost, it's necessary to match the https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelectorAll, which states that elements must be returned in document order.

Verification

example script is below

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
</head>
<body>
    <h1>List Order Test</h1>
    <ul id="test-list">
        <li>Item 0</li>
        <li>Item 1</li>
        <li>Item 2</li>
        <li>Item 3</li>
        <li>Item 4</li>
        <li>Item 5</li>
        <li>Item 6</li>
        <li>Item 7</li>
        <li>Item 8</li>
        <li>Item 9</li>
    </ul>
</body>
</html>
import { browser } from 'k6/browser';
import { check } from 'k6';

export const options = {
  scenarios: {
    ui: {
      executor: 'shared-iterations',
      options: {
        browser: {
          type: 'chromium',
        },
      },
    },
  },
};

export default async function () {
  const page = await browser.newPage();

  try {
    await page.goto('http://localhost:3000');

    const items = await page.$$('li');
    for (const item of items) {
        const text = await item.textContent();
        console.log(text);
    }
  } finally {
    await page.close();
  }
}

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

@shota3506 shota3506 requested a review from a team as a code owner October 14, 2025 15:33
@shota3506 shota3506 requested review from inancgumus and oleiade and removed request for a team October 14, 2025 15:33
inancgumus
inancgumus requested changes Oct 14, 2025
View reviewed changes
Copy link
Contributor

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution 🙇

Tomorrow, I'll continue reviewing this PR.

internal/js/modules/k6/browser/common/element_handle.go
els = append(els, el)
// DOM elements are stored with numeric indices ("0", "1", "2", ...)
// per JavaScript's array-like object specification.
idx, _ := strconv.Atoi(key)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should handle the error here.

internal/js/modules/k6/browser/common/element_handle.go
Comment on lines +1408 to +1411
els := make([]*ElementHandle, 0, len(indexedElems))
for _, ie := range indexedElems {
els = append(els, ie.elem)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to create another slice, as SortFunc sorts the slice in place.

@inancgumus inancgumus changed the title return elements in document order from QueryAll Match the Web API spec for QueryAll Oct 14, 2025
@shota3506 shota3506 temporarily deployed to azure-trusted-signing October 14, 2025 21:15 — with GitHub Actions Inactive
@shota3506 shota3506 temporarily deployed to azure-trusted-signing October 14, 2025 21:17 — with GitHub Actions Inactive
@shota3506 shota3506 changed the title Match the Web API spec for QueryAll return elements in document order from QueryAll Oct 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inancgumus inancgumus inancgumus requested changes

@oleiade oleiade Awaiting requested review from oleiade oleiade is a code owner automatically assigned from grafana/k6-core

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Selector page.$$ list order changes every time

2 participants

@shota3506 @inancgumus