[JavaScript] Move 'isDisplayed' atom to be typescript#17316
Open
AutomatedTester wants to merge 11 commits intotrunkfrom
Open
[JavaScript] Move 'isDisplayed' atom to be typescript#17316AutomatedTester wants to merge 11 commits intotrunkfrom
AutomatedTester wants to merge 11 commits intotrunkfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new TypeScript-based implementation of the isDisplayed/isShown atom, wires it into Bazel targets, and switches the Python binding’s packaged isDisplayed.js to use the new implementation.
Changes:
- Added a new TypeScript
isShownimplementation plus corresponding JS artifacts (is-displayed.js,is-displayed-global.js). - Added a new
is-displayed-typescriptfragment target and a selenium-webdriveris_displayed_typescriptmodule build target. - Updated the Python Bazel packaging step to copy the TypeScript-based fragment as
selenium/webdriver/remote/isDisplayed.js, and added new QUnit HTML tests for the TS variant.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
py/BUILD.bazel |
Switches Python’s packaged isDisplayed.js atom source to the new TS-based fragment output. |
javascript/selenium-webdriver/lib/atoms/BUILD.bazel |
Adds a build rule to wrap the TS-based fragment for Node/module consumption. |
javascript/atoms/typescript/is-displayed.ts |
Adds the new TypeScript source implementation of isShown. |
javascript/atoms/typescript/is-displayed.js |
Adds the JS artifact used as the fragment source for Bazel/copying. |
javascript/atoms/typescript/is-displayed-global.js |
Adds a global window.bot.dom.typescript.isShown build used by HTML tests. |
javascript/atoms/test/shown_typescript_test.html |
Adds a TS-focused clone of the existing shown_test.html. |
javascript/atoms/test/html5/shadow_dom_typescript_test.html |
Adds a TS-focused clone of the existing shadow-dom shown tests. |
javascript/atoms/fragments/BUILD.bazel |
Adds a new fragment target that copies in the TS-based implementation. |
javascript/atoms/BUILD.bazel |
Expands the atoms filegroup glob to include **/*.ts. |
a6a2ff6 to
ad8d8ed
Compare
ad8d8ed to
c45daee
Compare
AutomatedTester
added a commit
that referenced
this pull request
Apr 20, 2026
…ia Bazel - Fix invalid Bazel label in js_binary: entry_point must not use the ':' target prefix when the path contains '/'. Changed ':typescript/strip-trailing-semicolon.js' to the file-path form 'typescript/strip-trailing-semicolon.js'. - Add wrap-as-global.js Node script that wraps the compiled TypeScript atom output with the window.bot.dom.typescript.isShown global assignment (plus license header), mirroring the pattern of strip-trailing-semicolon.js. - Add 'wrap_as_global' js_binary and 'is-displayed-global' js_run_binary rules so is-displayed-global.js is generated from the same TypeScript source rather than being a separate hand-maintained copy that could drift. - Remove the hand-maintained javascript/atoms/typescript/is-displayed-global.js; it is now produced by the new Bazel rule. Fixes review comments in PR #17316.
3 tasks
AutomatedTester
added a commit
that referenced
this pull request
Apr 25, 2026
…ia Bazel - Fix invalid Bazel label in js_binary: entry_point must not use the ':' target prefix when the path contains '/'. Changed ':typescript/strip-trailing-semicolon.js' to the file-path form 'typescript/strip-trailing-semicolon.js'. - Add wrap-as-global.js Node script that wraps the compiled TypeScript atom output with the window.bot.dom.typescript.isShown global assignment (plus license header), mirroring the pattern of strip-trailing-semicolon.js. - Add 'wrap_as_global' js_binary and 'is-displayed-global' js_run_binary rules so is-displayed-global.js is generated from the same TypeScript source rather than being a separate hand-maintained copy that could drift. - Remove the hand-maintained javascript/atoms/typescript/is-displayed-global.js; it is now produced by the new Bazel rule. Fixes review comments in PR #17316.
AutomatedTester
added a commit
that referenced
this pull request
Apr 25, 2026
Replace all 'instanceof Element' (and related HTMLFormElement, HTMLMapElement, HTMLAreaElement) checks with nodeType-based checks so that isShown correctly handles Elements originating from a different JS realm (e.g., elements from an iframe). Closes review comment r3118809041 on PR #17316. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b9d3184 to
c21f959
Compare
AutomatedTester
added a commit
that referenced
this pull request
Apr 25, 2026
Port the getAttribute WebDriver atom from Google Closure (javascript/webdriver/atoms/attribute.js) to TypeScript following the pattern established for is-displayed in PR #17316. Changes: - javascript/atoms/typescript/get-attribute.ts: TypeScript IIFE implementation of the getAttribute atom. Drops IE compatibility (IE_DOC_PRE8/PRE9, getAttributeNode quirks), uses nodeType-based isElement() for cross-realm safety. Preserves all existing behavior: style → cssText, selected/checked state-based, href/src property resolution, spellcheck normalisation, boolean property list, PROPERTY_ALIASES (class→className, readonly→readOnly). - javascript/atoms/typescript/strip-trailing-semicolon.js: Node script that removes the trailing semicolon tsc adds after the IIFE, so the output can be injected as a JS expression. - javascript/atoms/typescript/wrap-get-attribute-as-global.js: Node script that wraps the compiled output as window.bot.dom.typescript.getAttribute for use in browser tests. - javascript/atoms/test/attribute_typescript_test.html: QUnit test page for the TypeScript atom covering named attrs, class, missing attrs, empty string, boolean attrs, selected/checked state, href/src URL resolution, style cssText, spellcheck, case insensitivity, expandos. - javascript/atoms/BUILD.bazel: Adds js_binary and js_run_binary rules to compile TypeScript → strip semicolon → wrap as global; updates the atoms filegroup to include .ts sources and the generated global. - javascript/atoms/fragments/BUILD.bazel: Adds copy_file rule get-attribute-typescript exposing the generated JS as a fragment. - py/BUILD.bazel: Switches the get-attribute copy_file to use //javascript/atoms/fragments:get-attribute-typescript.js instead of the Closure-compiled fragment. The old Closure get-attribute fragment is kept unchanged so that Ruby, Java, .NET, and JavaScript bindings are not affected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ia Bazel - Fix invalid Bazel label in js_binary: entry_point must not use the ':' target prefix when the path contains '/'. Changed ':typescript/strip-trailing-semicolon.js' to the file-path form 'typescript/strip-trailing-semicolon.js'. - Add wrap-as-global.js Node script that wraps the compiled TypeScript atom output with the window.bot.dom.typescript.isShown global assignment (plus license header), mirroring the pattern of strip-trailing-semicolon.js. - Add 'wrap_as_global' js_binary and 'is-displayed-global' js_run_binary rules so is-displayed-global.js is generated from the same TypeScript source rather than being a separate hand-maintained copy that could drift. - Remove the hand-maintained javascript/atoms/typescript/is-displayed-global.js; it is now produced by the new Bazel rule. Fixes review comments in PR #17316.
- Replace Element.closest('select') with a manual ancestor walk for
OPTION/OPTGROUP visibility, avoiding crashes in environments without
closest() support (e.g. IE11)
- Replace 'instanceof HTMLDetailsElement' with an isElement('DETAILS')
tagName check to avoid ReferenceError in browsers that do not define
HTMLDetailsElement
- Fix js_run_binary srcs to use explicit file label
'//javascript/atoms/fragments:is-displayed-typescript.js' instead of
the rule label to match the established pattern
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace all 'instanceof Element' (and related HTMLFormElement, HTMLMapElement, HTMLAreaElement) checks with nodeType-based checks so that isShown correctly handles Elements originating from a different JS realm (e.g., elements from an iframe). Closes review comment r3118809041 on PR #17316. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c21f959 to
9c5905f
Compare
- Cast elem to HTMLAreaElement when calling getAreaRelativeRect, since isElement() type guard only narrows to Element, not subtypes - Cast elem to Element before accessing parentNode to avoid TypeScript narrowing elem to never after the isElement(elem, 'BODY') false branch Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A <form> with a child <input name="tagName"> shadows the form's native tagName DOM property. Accessing form.tagName returns the input element (not the string "FORM"), causing isElement() to return false for the form and throw 'Argument to isShown must be of type Element'. Fix by reading tagName via the Element.prototype getter, which bypasses form child property shadowing while remaining safe for cross-realm elements (the native getter operates on the underlying DOM node). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…heck The TypeScript isElement() uses nodeType !== 1 (not instanceof Element), so a cross-realm <title> element passes the check and isShown() returns false rather than throwing. Update test accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+56
to
+61
| // Access tagName via the Element prototype getter to avoid property shadowing | ||
| // by form children: e.g. <form><input name="tagName"> shadows form.tagName. | ||
| var tagNameDescriptor = Object.getOwnPropertyDescriptor(Element.prototype, 'tagName'); | ||
| var rawTagName = tagNameDescriptor && typeof tagNameDescriptor.get === 'function' | ||
| ? tagNameDescriptor.get.call(asElement) | ||
| : asElement.tagName; |
Comment on lines
+10
to
+12
| <script type="text/javascript"> | ||
| goog.require('bot.dom'); | ||
| </script> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Related Issues
💥 What does this PR do?
Addiditive PR that moves Python code to use the newly created isDisplayed TypeScript implementation
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes