-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(controller): cap indexed options per dropdown container #649
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| import { beforeEach, describe, expect, it } from 'vitest' | ||
|
|
||
| import { flatTreeToString } from '../index' | ||
| import domTree from './index.js' | ||
|
|
||
| function setupSizes() { | ||
| Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { | ||
| configurable: true, | ||
| get() { | ||
| return 100 | ||
| }, | ||
| }) | ||
| Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { | ||
| configurable: true, | ||
| get() { | ||
| return 30 | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| function buildDropdown(html: string) { | ||
| return domTree({ | ||
| doHighlightElements: false, | ||
| viewportExpansion: -1, | ||
| interactiveBlacklist: [], | ||
| interactiveWhitelist: [], | ||
| }) as any | ||
| } | ||
|
|
||
| describe('dropdown option cap (#348)', () => { | ||
| beforeEach(() => { | ||
| setupSizes() | ||
| document.body.innerHTML = '' | ||
| }) | ||
|
|
||
| it('indexes at most 20 options per dropdown container', () => { | ||
| const options = Array.from( | ||
| { length: 25 }, | ||
| (_, i) => `<li class="el-select-dropdown__item" style="cursor:pointer">选项 ${i}</li>` | ||
| ) | ||
| document.body.innerHTML = ` | ||
| <div class="el-select-dropdown"> | ||
| <ul class="el-select-dropdown__list">${options.join('')}</ul> | ||
| </div> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const liNodes = Object.values(tree.map).filter((n: any) => n.tagName === 'li') | ||
| const indexed = liNodes.filter((n: any) => typeof n.highlightIndex === 'number') | ||
| expect(indexed.length).toBe(20) | ||
| }) | ||
|
|
||
| it('records dropped options on the container', () => { | ||
| const options = Array.from( | ||
| { length: 25 }, | ||
| (_, i) => `<li class="el-select-dropdown__item" style="cursor:pointer">选项 ${i}</li>` | ||
| ) | ||
| document.body.innerHTML = ` | ||
| <div class="el-select-dropdown"> | ||
| <ul class="el-select-dropdown__list">${options.join('')}</ul> | ||
| </div> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const container = Object.values(tree.map).find( | ||
| (n: any) => n.tagName === 'div' && n.extra?.droppedOptions === 5 | ||
| ) | ||
| expect(container).toBeDefined() | ||
| }) | ||
|
|
||
| it('renders a folded-options hint in the simplified HTML', () => { | ||
| const options = Array.from( | ||
| { length: 25 }, | ||
| (_, i) => `<li class="el-select-dropdown__item" style="cursor:pointer">选项 ${i}</li>` | ||
| ) | ||
| document.body.innerHTML = ` | ||
| <div class="el-select-dropdown"> | ||
| <ul class="el-select-dropdown__list">${options.join('')}</ul> | ||
| </div> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const html = flatTreeToString(tree) | ||
| expect(html).toContain('5 more option(s) not shown') | ||
| // only the first 20 options carry indexes | ||
| expect(html).toContain('[19]<li >选项 19') | ||
| expect(html).not.toContain('选项 24') | ||
| }) | ||
|
|
||
| it('does not cap dropdowns with few options', () => { | ||
| const options = Array.from( | ||
| { length: 6 }, | ||
| (_, i) => `<li class="el-select-dropdown__item" style="cursor:pointer">选项 ${i}</li>` | ||
| ) | ||
| document.body.innerHTML = ` | ||
| <div class="el-select-dropdown"> | ||
| <ul class="el-select-dropdown__list">${options.join('')}</ul> | ||
| </div> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const liNodes = Object.values(tree.map).filter((n: any) => n.tagName === 'li') | ||
| expect(liNodes.filter((n: any) => typeof n.highlightIndex === 'number').length).toBe(6) | ||
| const html = flatTreeToString(tree) | ||
| expect(html).not.toContain('not shown') | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import { beforeEach, describe, expect, it } from 'vitest' | ||
|
|
||
| import domTree from './index.js' | ||
|
|
||
| function setupSizes() { | ||
| Object.defineProperty(HTMLElement.prototype, 'offsetWidth', { | ||
| configurable: true, | ||
| get() { | ||
| return 100 | ||
| }, | ||
| }) | ||
| Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { | ||
| configurable: true, | ||
| get() { | ||
| return 30 | ||
| }, | ||
| }) | ||
| } | ||
|
|
||
| function buildDropdown(html: string) { | ||
| return domTree({ | ||
| doHighlightElements: false, | ||
| viewportExpansion: -1, | ||
| interactiveBlacklist: [], | ||
| interactiveWhitelist: [], | ||
| }) as any | ||
| } | ||
|
|
||
| function getIndexedOptions(tree: any) { | ||
| return Object.values(tree.map) | ||
| .filter((n: any) => n.tagName === 'li' && typeof n.highlightIndex === 'number') | ||
| .sort((a: any, b: any) => a.highlightIndex - b.highlightIndex) | ||
| } | ||
|
|
||
| describe('dropdown option indexing (#519)', () => { | ||
| beforeEach(() => { | ||
| setupSizes() | ||
| document.body.innerHTML = '' | ||
| }) | ||
|
|
||
| it('indexes enabled dropdown options', () => { | ||
| document.body.innerHTML = ` | ||
| <div class="el-select-dropdown"> | ||
| <ul class="el-select-dropdown__list"> | ||
| <li class="el-select-dropdown__item" style="cursor:pointer">选项1</li> | ||
| <li class="el-select-dropdown__item" style="cursor:pointer">选项2</li> | ||
| <li class="el-select-dropdown__item" style="cursor:pointer">选项3</li> | ||
| </ul> | ||
| </div> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const indexed = getIndexedOptions(tree) | ||
| expect(indexed.length).toBe(3) | ||
| }) | ||
|
|
||
| it('indexes disabled dropdown options so the full list is visible to the LLM', () => { | ||
| document.body.innerHTML = ` | ||
| <div class="el-select-dropdown"> | ||
| <ul class="el-select-dropdown__list"> | ||
| <li class="el-select-dropdown__item" style="cursor:pointer">闸片产线</li> | ||
| <li class="el-select-dropdown__item is-disabled" style="cursor:not-allowed">落料车间</li> | ||
| <li class="el-select-dropdown__item" style="cursor:pointer">机加车间</li> | ||
| </ul> | ||
| </div> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const indexed = getIndexedOptions(tree) | ||
| expect(indexed.length).toBe(3) | ||
| }) | ||
|
|
||
| it('still excludes disabled buttons outside dropdowns', () => { | ||
| document.body.innerHTML = ` | ||
| <button style="cursor:not-allowed" disabled>保存</button> | ||
| ` | ||
| const tree = buildDropdown(document.body.innerHTML) | ||
| const buttons = Object.values(tree.map).filter((n: any) => n.tagName === 'button') | ||
| expect(buttons.every((n: any) => n.highlightIndex === undefined)).toBe(true) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -687,6 +687,54 @@ export default ( | |
| ) | ||
| } | ||
|
|
||
| /** | ||
| * @edit dropdown/menu option detection | ||
| * li/option elements (or role=option/menuitem/listitem) inside a dropdown or menu | ||
| * container are treated as indexable even when visually disabled (cursor: not-allowed), | ||
| * so the LLM sees the complete option list. | ||
| */ | ||
| const DROPDOWN_OPTION_ROLES = new Set([ | ||
| 'option', | ||
| 'menuitem', | ||
| 'menuitemradio', | ||
| 'menuitemcheckbox', | ||
| 'listitem', | ||
| ]) | ||
| const DROPDOWN_CONTAINER_SELECTOR = [ | ||
| '[role="listbox"]', | ||
| '[role="menu"]', | ||
| '[role="menubar"]', | ||
| 'select', | ||
| '.el-select-dropdown', | ||
| '.el-dropdown-menu', | ||
| '[data-toggle="dropdown"]', | ||
| ].join(', ') | ||
|
|
||
| function isDropdownOptionElement(element) { | ||
| if (!element || element.nodeType !== Node.ELEMENT_NODE) return false | ||
| const tagName = element.tagName.toLowerCase() | ||
| const role = element.getAttribute('role') | ||
| if (tagName !== 'li' && tagName !== 'option' && !(role && DROPDOWN_OPTION_ROLES.has(role))) { | ||
| return false | ||
| } | ||
| return Boolean(element.closest(DROPDOWN_CONTAINER_SELECTOR)) | ||
| } | ||
|
|
||
| /** | ||
| * @edit cap dropdown options per container (#348) | ||
| * A select/dropdown with hundreds of options would blow up the LLM payload; | ||
| * index at most MAX_DROPDOWN_OPTIONS_PER_CONTAINER options and fold the rest | ||
| * into a hint on the container. | ||
| */ | ||
| const MAX_DROPDOWN_OPTIONS_PER_CONTAINER = 20 | ||
| const dropdownOptionCounts = new WeakMap() // container element -> indexed option count | ||
|
|
||
| function getDropdownOptionContainer(element) { | ||
| if (!element || element.nodeType !== Node.ELEMENT_NODE) return null | ||
| if (!isDropdownOptionElement(element)) return null | ||
| return element.closest(DROPDOWN_CONTAINER_SELECTOR) | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an element is interactive. | ||
| * | ||
|
|
@@ -711,6 +759,15 @@ export default ( | |
| return true // Skip whitelisted elements | ||
| } | ||
|
|
||
| /** | ||
| * @edit dropdown options should stay indexable even when disabled, | ||
| * otherwise the LLM cannot see or select the full option list | ||
| * (e.g. Element UI / Avue selects with disabled options). | ||
| */ | ||
| if (isDropdownOptionElement(element)) { | ||
| return true | ||
| } | ||
|
|
||
| // Cache the tagName and style lookups | ||
| const tagName = element.tagName.toLowerCase() | ||
| const style = getCachedComputedStyle(element) | ||
|
|
@@ -1635,6 +1692,24 @@ export default ( | |
| if (nodeData.isVisible) { | ||
| nodeData.isTopElement = isTopElement(node) | ||
|
|
||
| /** | ||
| * @edit cap dropdown options per container (#348) | ||
| * A dropdown with hundreds of options would blow up the LLM payload; | ||
| * index at most MAX_DROPDOWN_OPTIONS_PER_CONTAINER options per container | ||
| * and fold the excess into a hint on the container. | ||
| */ | ||
| if (isDropdownOptionElement(node)) { | ||
| const container = getDropdownOptionContainer(node) | ||
| const used = dropdownOptionCounts.get(container) || 0 | ||
| if (used >= MAX_DROPDOWN_OPTIONS_PER_CONTAINER) { | ||
| addExtraData(container, { | ||
| droppedOptions: (extraData.get(container)?.droppedOptions || 0) + 1, | ||
| }) | ||
| return null // Skip excess options entirely (text included) | ||
| } | ||
| dropdownOptionCounts.set(container, used + 1) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| // Special handling for ARIA menu containers - check interactivity even if not top element | ||
| const role = node.getAttribute('role') | ||
| const isMenuContainer = role === 'menu' || role === 'menubar' || role === 'listbox' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a custom dropdown encodes disabled options only via class/cursor (for example Element UI's
.is-disabledpluscursor:not-allowed), this early return bypasses the existing non-interactive cursor and disabled checks and gives the item a normal clickable index. Since the simplified DOM does not include class/style by default, the model cannot tell that index is not selectable, andclick_element_by_indexreports success even if the UI ignores the disabled click; include a disabled marker or render these as unindexed text instead.Useful? React with 👍 / 👎.