Fix(uia): resolve selection retrieval failure with tabindex parent#14
Fix(uia): resolve selection retrieval failure with tabindex parent#14sdp-ncd wants to merge 2 commits into0xfullex:mainfrom
Conversation
|
@0xfullex I tested this fix, and it works correctly—it retrieves the content of child elements within the parent element that have However, it introduced a new issue: it copies some icons from the webpage, like “\n”. Do you think this should be handled at the C++ layer or the JavaScript layer? I believe handling it at the JavaScript layer would be more reasonable. 我这边尝试了一下,这个修复是正确的,能够获取父元素包含tabindex=...的子元素内容。 但是引发了一个新的问题:会把网页中的一些图标复制出来,类似于“\n”,因为这些元素返回给 UIA 的内容就是这个字符,表示这是一个"嵌入式对象" 你觉得是在c++层做处理呢?还是在js层做处理呢?在js层做处理可以由客户端自定义进行替换,例如替换成 但是对于系统原生的 ctrl+c 实际上只是简单地直接略过这个图标 |
|
最新的代码已经在c++原生层做处理了,最终的复制效果和直接 ctrl+c 一致,不会在文本中包含[obj] |
0xfullex
left a comment
There was a problem hiding this comment.
PR Review: Fix(uia): resolve selection retrieval failure with tabindex parent
Is this a real problem?
Yes. In modern browsers (Chrome/Edge), setting tabindex="0" on a container element causes it to receive UI Automation focus instead of the document root. Since these containers typically don't implement TextPattern, the original code fails to retrieve the selection. This is a well-known UIA focus mismatch issue.
Solution Assessment
The ancestor walk-up mechanism is the correct approach — walking up the UIA tree to find an ancestor (usually the Document element) that supports TextPattern and holds the active selection. The TryGetTextFromElement lambda and the ElementFromPoint-first strategy are solid improvements that reduce code duplication and improve accuracy for mouse-based selections. The RemoveObjectReplacementChars handling for U+FFFC is also correct and consistent with native Ctrl+C behavior.
Issues Found
1. SetTextRangeCoordinates return value ignored (Severity: High)
Before:
result = SetTextRangeCoordinates(pRange, selectionInfo); // failure = not successfulAfter:
SetTextRangeCoordinates(pRange, selectionInfo); // ignored, returns true regardlessThis could return text with invalid coordinates to downstream consumers.
2. VT_ARRAY selection handling removed (Severity: Medium)
The original handled IAccessible::get_accSelection returning VT_ARRAY (multi-selection scenarios). This entire code path was removed without explanation. Even if uncommon, this is a functionality regression.
3. ExpandToEnclosingUnit fallback removed (Severity: Medium)
The original had a fallback that called pDocRange->ExpandToEnclosingUnit(TextUnit_Document) when the initial DocumentRange query didn't find an active selection. This fallback was removed, which could cause regressions with certain UIA providers that don't return the document range directly.
4. get_accName / get_accValue fallback behavior changed (Severity: Low)
Before: Checks SysStringLen(bstr) > 0 after get_accName; if empty, frees bstr and falls through to get_accValue.
After: Uses || short-circuit — if get_accName returns a non-null but empty/unhelpful BSTR, it won't attempt get_accValue.
5. uia_control_type semantics changed (Severity: Low)
Previously reflected the focused element's ControlType; now reflects the ancestor element that successfully returned text (e.g., Document instead of the originally focused element). Could affect downstream logic that depends on this value.
Summary
| Aspect | Verdict |
|---|---|
| Problem validity | Real and meaningful |
| Core approach (Walk Up) | Correct |
| ElementFromPoint priority | Good improvement |
| RemoveObjectReplacementChars | Correct |
| Code structure | Improved (less duplication) |
| Feature completeness | Regressions (issues 1-3) |
The core fix is valuable. Consider restoring the SetTextRangeCoordinates return value check, the VT_ARRAY handling, and the ExpandToEnclosingUnit fallback before merging.
|
I'll fix these issues. |
[WIP] tested but not code reviewed
Description
This PR fixes an issue where text selection retrieval via UI Automation fails when the selected text resides within a parent element having
tabindex="0".Problem
In modern web browsers (Chrome/Edge), setting
tabindex="0"on a container element (like a<div>) causes it to receive UI Automation focus instead of the specific text node or the document root. Since these container elements often do not implement theTextPatterninterface (or delegate selection management to the document root), the previous implementation would fail to retrieve the selection, returning an empty result.Additionally, the
ElementFromPointstrategy would often target leaf nodes (like<span>) that also lackTextPatternsupport, missing the actual selection managed by ancestor nodes.Solution
TextPatternand contains the active selection.TryGetTextFromElement) to reduce code duplication and improve readability.ElementFromPointstrategy with the new walk-up logic to better handle mouse-based selections.Changes
SelectionHook::GetTextViaUIAutomationto include ancestor traversal loops.TryGetTextFromElementhelper for consistent pattern checking.Impact
This ensures reliable text selection retrieval in complex HTML structures involving
tabindexand nested elements, significantly improving compatibility with web-based applications.