fix: update xpath selector selection logic yet again #1135
Conversation
app/renderer/util.js
Outdated
| // even if this node name is unique, if it's the root node, we don't want to refer to it using | ||
| // '//' but rather '/' | ||
| if (!domNode?.parentNode?.tagName) { | ||
| xpath = `/${domNode.tagName}`; | ||
| } |
There was a problem hiding this comment.
I'm not sure how useful this condition is. From the apps I checked, Android always has two android.widget.FrameLayout as the roots, so isUnique will be false anyway; and iOS has XCUIElementTypeApplication with filled out name and label fields, so the xpath generator will rely on those, and not even reach this base case. So currently I haven't found a usecase for this.
Also, CI is failing here because we can't use optional chaining (Parcel v1 limitation). It probably needs to be something like if (!(domNode.parentNode && domNode.parentNode.tagName)).
Finally, if we really must keep the condition, it should probably be moved inside the isUnique block on the next line.
There was a problem hiding this comment.
It really matters for our tests, so I don't have to change a million lines of test code, and I think it might matter for some other hierarchies that come from other platforms. But I'll fix the optional chaining issue.
jlipps
force-pushed
the
jlipps/xpath-suggestions
branch
from
106b61e
to
eb01e75
Compare
this PR primarily removes class/type from xpath uniqueness attributes. instead adds logic to look for unique node names and refer to those if necessary (since class/type are always the same as node names)