selenium Refactor isDisplayed atom to remove Closure library code

User description

The Closure library adds a lot of complexity to the compiled and minified atoms, including for isDisplayed. Additionally, the web platform has evolved in the time since the atoms were originally written, with additional methods added that help simplify the atom. This commit removes (nearly) all of the Closure code from the specific atom used by the isDisplayed method in the various language bindings.

Thanks for contributing to Selenium! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines. Avoid large PRs, help reviewers by making them as simple and short as possible.

Motivation and Context

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[x] I have read the contributing document.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[x] All new and existing tests passed.

PR Type

Enhancement, Bug fix

Description

Refactored isDisplayed atom to remove Closure library dependencies.
Simplified visibility checks using modern web platform methods.
Improved handling of <map> and <area> elements for visibility determination.
Enhanced overflow and size-based visibility logic for better accuracy.

Changes walkthrough 📝

Relevant files

Enhancement

dom.js

Refactored `isDisplayed` atom for modern web standards

javascript/atoms/dom.js

Replaced Closure library methods with modern JavaScript APIs.

Added new helper functions for and visibility checks.

Enhanced logic for handling overflow and size-based visibility.

Improved compatibility with shadow DOM and modern web standards.

+219/-69

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

Mar 28 '25 13:03 jimevans

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: diemol
:x: jimevanssfdc
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Mar 28 '25 13:03 CLAassistant

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Potential Bug

The getAreaRelativeRect function doesn't properly parse coordinates as numbers, which could lead to incorrect calculations. The coordinates are split from a string but not converted to numbers before mathematical operations.

const getAreaRelativeRect = (area) => {
  const shape = area.shape.toLowerCase();
  const coords = area.coords.split(',');
  if (shape == 'rect' && coords.length == 4) {
    const [x, y] = coords;
    return new DOMRect(x, y,coords[2] - x, coords[3] - y);
  } else if (shape == 'circle' && coords.length == 3) {
    const [centerX, centerY, radius] = coords;
    return new DOMRect(centerX - radius, centerY - radius, 2 * radius, 2 * radius);
  } else if (shape == 'poly' && coords.length > 2) {
    let [minX, minY] = coords, maxX = minX, maxY = minY;
    for (var i = 2; i + 1 < coords.length; i += 2) {
      minX = Math.min(minX, coords[i]);
      maxX = Math.max(maxX, coords[i]);
      minY = Math.min(minY, coords[i + 1]);
      maxY = Math.max(maxY, coords[i + 1]);
    }
    return new DOMRect(minX, minY, maxX - minX, maxY - minY);
  }
  return new DOMRect();
};

Edge Case Handling

The overflow detection logic in checkIsHiddenByOverflow might not handle all edge cases correctly, particularly with nested scrollable containers and fixed position elements. The scroll position calculation could be improved.

const checkIsHiddenByOverflow = (elem, style) => {
  const htmlElement = elem.ownerDocument.documentElement;

  const getNearestOverflowAncestor = (e, style) => {
    const elementPosition = style.getPropertyValue('position');
    const canBeOverflowed = (container) => {
      const containerStyle = getComputedStyle(container);
      if (container === htmlElement) {
        return true;
      }
      const containerDisplay = containerStyle.getPropertyValue('display');
      if (containerDisplay.startsWith('inline') || containerDisplay === 'contents') {
        return false;
      }
      const containerPosition = containerStyle.getPropertyValue('position');
      if (elementPosition === 'absolute' && containerPosition === 'static') {
        return false;
      }
      return true;
    };
    if (elementPosition === 'fixed') {
      return e === htmlElement ? null : htmlElement;
    }
    let container = e.parentElement;
    while (container && !canBeOverflowed(container)) {
      container = container.parentElement;
    }
    return container;
  };

  // Walk up the tree, examining each ancestor capable of displaying
  // overflow.
  let parentElement = getNearestOverflowAncestor(elem, style);
  while (parentElement) {
    const parentStyle = getComputedStyle(parentElement);
    const parentOverflowX = parentStyle.getPropertyValue('overflow-x');
    const parentOverflowY = parentStyle.getPropertyValue('overflow-y');

    // If the container has overflow:visible, the element cannot be hidden in its overflow.
    if (parentOverflowX !== 'visible' || parentOverflowY !== 'visible') {
      const parentRect = parentElement.getBoundingClientRect();

      // Zero-sized containers without overflow:visible hide all descendants.
      if (parentRect.width === 0 || parentRect.height === 0) {
        return true;
      }

      const elementRect = elem.getBoundingClientRect();

      // Check "underflow": if an element is to the left or above the container
      // and overflow is "hidden" in the proper direction, the element is hidden.
      const isLeftOf = elementRect.x + elementRect.width < parentRect.x;
      const isAbove = elementRect.y + elementRect.height < parentRect.y;
      if ((isLeftOf && parentOverflowX === 'hidden') ||
          (isAbove && parentOverflowY === 'hidden')) {
        return true;
      }

      // Check "overflow": if an element is to the right or below a container
      // and overflow is "hidden" in the proper direction, the element is hidden.
      const isRightOf = elementRect.x >= parentRect.x + parentRect.width;
      const isBelow = elementRect.y >= parentRect.y + parentRect.height;
      if ((isRightOf && parentOverflowX === 'hidden') ||
          (isBelow && parentOverflowY === 'hidden')) {
        return true;
      } else if ((isRightOf && parentOverflowX !== 'visible') ||
          (isBelow && parentOverflowY !== 'visible')) {
        // Special case for "fixed" elements: whether it is hidden by
        // overflow depends on the scroll position of the parent element
        if (style.getPropertyValue('position') === 'fixed') {
          const scrollPosition = parentElement.tagName === 'HTML' ?
            {
              x: parentElement.ownerDocument.scrollingElement.scrollLeft,
              y: parentElement.ownerDocument.scrollingElement.scrollTop
            } :
            {
              x: parentElement.scrollLeft,
              y: parentElement.scrollTop
            };
          if ((elementRect.x >= htmlElement.scrollWidth - scrollPosition.x) ||
              (elementRect.y >= htmlElement.scrollHeight - scrollPosition.y)) {
            return true;
          }
        }
      }
    }
    parentElement = getNearestOverflowAncestor(parentElement, parentStyle);
  }
  return false;
};

Browser Compatibility

The use of checkVisibility() method may not be supported in all browsers that Selenium targets. This could cause compatibility issues if not properly feature-detected or polyfilled.

const checkElementVisibility = (elem) => {
  return elem.checkVisibility({
    visibilityProperty: true,
    opacityProperty: !ignoreOpacity,
    contentVisibilityAuto: true
  });
};

Mar 28 '25 13:03 qodo-merge-pro[bot]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Parse string coordinates to numbers Fix the parsing of coordinates in the rectangle shape handler. The coordinates are currently being used as strings, which will cause incorrect calculations. Parse them to numbers first. javascript/atoms/dom.js [467-468] `-const [x, y] = coords; -return new DOMRect(x, y,coords[2] - x, coords[3] - y); +const [x, y] = coords.map(Number); +return new DOMRect(x, y, Number(coords[2]) - x, Number(coords[3]) - y);` [ ] Apply this suggestion Suggestion importance[1-10]: 9 __ Why: This is a critical fix. Without parsing the coordinates to numbers, JavaScript will treat them as strings, leading to string concatenation instead of numeric subtraction, causing incorrect rectangle calculations and potential bugs in element visibility detection.	High
	Convert string coordinates to numbers The circle coordinates need to be parsed as numbers before performing arithmetic operations. Without parsing, JavaScript will perform string concatenation instead of numeric subtraction. javascript/atoms/dom.js [470-471] `-const [centerX, centerY, radius] = coords; +const [centerX, centerY, radius] = coords.map(Number); return new DOMRect(centerX - radius, centerY - radius, 2 * radius, 2 * radius);` [ ] Apply this suggestion Suggestion importance[1-10]: 9 __ Why: This is a critical fix for the circle shape handler. Without converting the string coordinates to numbers, arithmetic operations will result in string concatenation instead of numeric calculations, leading to incorrect DOMRect dimensions.	High
	Convert polygon coordinates to numbers The polygon coordinates need to be parsed as numbers for proper comparison. Using `Math.min` and `Math.max` with string values can lead to incorrect results due to string comparison rules. javascript/atoms/dom.js [473-479] -let [minX, minY] = coords, maxX = minX, maxY = minY; +let [minX, minY] = coords.map(Number), maxX = minX, maxY = minY; for (var i = 2; i + 1 < coords.length; i += 2) { - minX = Math.min(minX, coords[i]); - maxX = Math.max(maxX, coords[i]); - minY = Math.min(minY, coords[i + 1]); - maxY = Math.max(maxY, coords[i + 1]); + minX = Math.min(minX, Number(coords[i])); + maxX = Math.max(maxX, Number(coords[i])); + minY = Math.min(minY, Number(coords[i + 1])); + maxY = Math.max(maxY, Number(coords[i + 1])); } [ ] Apply this suggestion Suggestion importance[1-10]: 9 __ Why: This is a critical fix for polygon coordinate handling. String comparisons in Math.min/max will produce incorrect results when determining bounding rectangles, potentially causing visibility detection failures for polygon-shaped area elements.	High
[ ] Update

Mar 28 '25 13:03 qodo-merge-pro[bot]

@AutomatedTester @shs96c Here's the patch we paired on at SeConf in Valencia. Feel free to merge at your discretion.

Mar 28 '25 13:03 jimevans

@jimevans you have to sign the CLA for @jimevanssfdc

Apr 08 '25 19:04 titusfortner

This is breaking a few tests in Ruby and Python. https://github.com/SeleniumHQ/selenium/actions/runs/15156693114/job/42613646313?pr=15528#step:18:748

  1) Selenium::WebDriver::Element gets displayed
     Failure/Error: expect(driver.find_element(class: 'header')).to be_displayed

     Selenium::WebDriver::Error::JavascriptError:
       javascript error: a.a is not a function
         (Session info: chrome=136.0.7103.94)

May 21 '25 08:05 diemol

selenium selenium copied to clipboard

Refactor isDisplayed atom to remove Closure library code

User description

Motivation and Context

Types of changes

Checklist

PR Type

Description

Changes walkthrough 📝

PR Reviewer Guide 🔍

PR Code Suggestions ✨

selenium
selenium copied to clipboard