Skip to content

[js][py] Convert getAttribute atom from Closure to TypeScript#17370

Open
AutomatedTester wants to merge 2 commits intotrunkfrom
atom-getattribute
Open

[js][py] Convert getAttribute atom from Closure to TypeScript#17370
AutomatedTester wants to merge 2 commits intotrunkfrom
atom-getattribute

Conversation

@AutomatedTester
Copy link
Copy Markdown
Member

🔗 Related Issues

💥 What does this PR do?

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.

🔧 Implementation Notes

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): copilot
    • What was generated:
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

Copilot AI review requested due to automatic review settings April 22, 2026 11:35
@selenium-ci selenium-ci added C-py Python Bindings B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations labels Apr 22, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Ports the WebDriver getAttribute atom from the Closure-based implementation to a TypeScript IIFE, and wires Bazel + Python to consume the new generated fragment while adding a dedicated QUnit test page for the TS version.

Changes:

  • Added javascript/atoms/typescript/get-attribute.ts plus Bazel rules to compile it with tsc and post-process the output for fragment injection and browser-global testing.
  • Added small Node utilities to strip the trailing semicolon from the compiled IIFE output and to wrap the compiled atom as window.bot.dom.typescript.getAttribute for browser tests.
  • Updated py/BUILD.bazel to use the new TS-generated atom fragment, and added a new QUnit HTML test page for the TS atom.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
javascript/atoms/typescript/get-attribute.ts New TypeScript implementation of the getAttribute atom as an IIFE returning the callable function.
javascript/atoms/typescript/strip-trailing-semicolon.js Post-processing tool to make the compiled output usable as a JS expression by removing trailing ;.
javascript/atoms/typescript/wrap-get-attribute-as-global.js Post-processing tool to expose the compiled atom as a browser global for QUnit HTML test pages.
javascript/atoms/BUILD.bazel Adds tsc compilation and post-processing pipeline; updates atoms filegroup to include TS sources and the generated global.
javascript/atoms/fragments/BUILD.bazel Exposes the generated TS fragment via copy_file for consumption by bindings.
javascript/atoms/test/attribute_typescript_test.html New QUnit test page validating the TypeScript getAttribute atom behavior.
py/BUILD.bazel Switches Python’s getAttribute.js copy to the new TS fragment output.

Comment thread javascript/atoms/typescript/get-attribute.ts Outdated
AutomatedTester and others added 2 commits April 25, 2026 13:01
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>
…ject

Without this fix, event handler properties (e.g. onclick) returned by
getProperty() would not be treated as objects, causing the atom to fall
through to String(property) and return the stringified function source
instead of falling back to getAttribute().

Also adds a regression test for event handler attributes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 25, 2026 12:01
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

"false",
"--outFile",
"$(rootpath :typescript/get-attribute.compiled.js)",
"$(rootpath typescript/get-attribute.ts)",
Copy link

Copilot AI Apr 25, 2026

Choose a reason for hiding this comment

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

$(rootpath typescript/get-attribute.ts) is not a valid Bazel label (missing :), and will cause the js_run_binary action to fail label expansion. Use a proper file label for the TS source (e.g., :typescript/get-attribute.ts, or a fully-qualified label) when passing it to tsc.

Suggested change
"$(rootpath typescript/get-attribute.ts)",
"$(rootpath :typescript/get-attribute.ts)",

Copilot uses AI. Check for mistakes.
Comment thread javascript/atoms/test/attribute_typescript_test.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations C-py Python Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants