From f2b1fcd581c88a01946faff831b9e077739965ee Mon Sep 17 00:00:00 2001 From: Sumit Mansukhlal Savaliya Date: Sun, 26 Nov 2023 21:50:48 -0400 Subject: [PATCH] fix: removed code smells and refactored project structure --- .../selenium/htmlunit/HtmlUnitAlert.java | 28 +++++----- .../htmlunit/HtmlUnitElementFinder.java | 24 +++++---- .../htmlunit/HtmlUnitTargetLocator.java | 15 ++++-- .../selenium/htmlunit/HtmlUnitWebElement.java | 54 +++++++++++-------- .../htmlunit/junit/BrowserRunner.java | 2 +- 5 files changed, 74 insertions(+), 49 deletions(-) diff --git a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitAlert.java b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitAlert.java index 27198c6f..cb3414af 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitAlert.java +++ b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitAlert.java @@ -44,7 +44,7 @@ public class HtmlUnitAlert implements Alert { private final HtmlUnitDriver driver_; private AlertHolder holder_; - private boolean quitting_; + private boolean autoDismissOnQuit; private final Lock lock_ = new ReentrantLock(); private final Condition condition_ = lock_.newCondition(); private WebWindow webWindow_; @@ -60,7 +60,7 @@ public class HtmlUnitAlert implements Alert { } private void alertHandler(final Page page, final String message) { - if (quitting_) { + if (autoDismissOnQuit) { return; } webWindow_ = page.getEnclosingWindow(); @@ -69,7 +69,7 @@ private void alertHandler(final Page page, final String message) { } private boolean confirmHandler(final Page page, final String message) { - if (quitting_) { + if (autoDismissOnQuit) { return false; } webWindow_ = page.getEnclosingWindow(); @@ -83,12 +83,7 @@ private void awaitCondition() { lock_.lock(); try { if (driver_.isProcessAlert()) { - try { - condition_.await(5, TimeUnit.SECONDS); - } - catch (final InterruptedException e) { - throw new RuntimeException(e); - } + waitFor(5); } } finally { @@ -96,8 +91,17 @@ private void awaitCondition() { } } + private void waitFor(int time) { + try { + condition_.await(time, TimeUnit.SECONDS); + } + catch (final InterruptedException e) { + throw new RuntimeException(e); + } + } + private String promptHandler(final Page page, final String message, final String defaultMessage) { - if (quitting_) { + if (autoDismissOnQuit) { return null; } webWindow_ = page.getEnclosingWindow(); @@ -108,7 +112,7 @@ private String promptHandler(final Page page, final String message, final String } private boolean onbeforeunloadHandler(final Page page, final String returnValue) { - if (quitting_) { + if (autoDismissOnQuit) { return true; } webWindow_ = page.getEnclosingWindow(); @@ -123,7 +127,7 @@ WebWindow getWebWindow() { } public void setAutoAccept(final boolean autoAccept) { - this.quitting_ = autoAccept; + this.autoDismissOnQuit = autoAccept; } public void handleBrowserCapabilities(final Capabilities capabilities) { diff --git a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitElementFinder.java b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitElementFinder.java index 285c510e..90b95abf 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitElementFinder.java +++ b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitElementFinder.java @@ -124,7 +124,7 @@ public List findElements(final HtmlUnitDriver driver, final By locat } final List allElements = ((HtmlPage) lastPage).getElementsById(getValue(locator)); - return convertRawDomElementsToWebElements(driver, allElements); + return HtmlUnitElementUtils.convertRawDomElementsToWebElements(driver, allElements); } @Override @@ -150,7 +150,7 @@ public List findElements(final HtmlUnitDriver driver, final By locat } final List allElements = ((HtmlPage) lastPage).getElementsByName(getValue(locator)); - return convertRawDomElementsToWebElements(driver, allElements); + return HtmlUnitElementUtils.convertRawDomElementsToWebElements(driver, allElements); } @Override @@ -489,7 +489,7 @@ public WebElement findElement(final HtmlUnitWebElement element, final By locator // selector is therefore // invalid throw new InvalidSelectorException( - String.format(INVALIDSELECTIONERROR, value, node.getClass().toString())); + String.format(INVALIDSELECTIONERROR, value, node.getClass())); } @Override @@ -515,7 +515,7 @@ public List findElements(final HtmlUnitWebElement element, final By // selector is // therefore invalid throw new InvalidSelectorException( - String.format(INVALIDSELECTIONERROR, value, e.getClass().toString())); + String.format(INVALIDSELECTIONERROR, value, e.getClass())); } } return toReturn; @@ -565,14 +565,18 @@ protected static String getValue(final By locator) { } } - private static List convertRawDomElementsToWebElements( - final HtmlUnitDriver driver, final List nodes) { - final List toReturn = new ArrayList<>(nodes.size()); + public static class HtmlUnitElementUtils{ - for (final DomElement node : nodes) { - toReturn.add(driver.toWebElement(node)); + public static List convertRawDomElementsToWebElements( + final HtmlUnitDriver driver, final List nodes) { + final List toReturn = new ArrayList<>(nodes.size()); + + for (final DomElement node : nodes) { + toReturn.add(driver.toWebElement(node)); + } + + return toReturn; } - return toReturn; } } diff --git a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitTargetLocator.java b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitTargetLocator.java index 3e72eb3d..adde76a3 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitTargetLocator.java +++ b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitTargetLocator.java @@ -187,12 +187,14 @@ public WebElement activeElement() { @Override public Alert alert() { final HtmlUnitAlert alert = driver_.getAlert(); + final int SLEEP_TIME = 50; + final int NUMBER_OF_TRIES = 5; if (!alert.isLocked()) { - for (int i = 0; i < 5; i++) { + for (int i = 0; i < NUMBER_OF_TRIES; i++) { if (!alert.isLocked()) { try { - Thread.sleep(50); + Thread.sleep(SLEEP_TIME); } catch (final InterruptedException e) { throw new RuntimeException(e); @@ -208,13 +210,18 @@ public Alert alert() { final WebWindow alertWindow = alert.getWebWindow(); final WebWindow currentWindow = driver_.getCurrentWindow().getWebWindow(); - if (alertWindow != currentWindow && !isChild(currentWindow, alertWindow) - && !isChild(alertWindow, currentWindow)) { + if (isAlertTimedOut(alertWindow , currentWindow)) { throw new TimeoutException(); } + return alert; } + private static boolean isAlertTimedOut(WebWindow alertWindow , WebWindow currentWindow) { + return (alertWindow != currentWindow && !isChild(currentWindow, alertWindow) + && !isChild(alertWindow, currentWindow)); + } + private static boolean isChild(final WebWindow parent, final WebWindow potentialChild) { for (WebWindow child = potentialChild; child != null; child = child.getParentWindow()) { if (child == parent) { diff --git a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitWebElement.java b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitWebElement.java index 8d1b7763..e3210503 100644 --- a/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitWebElement.java +++ b/src/main/java/org/openqa/selenium/htmlunit/HtmlUnitWebElement.java @@ -81,13 +81,6 @@ */ public class HtmlUnitWebElement implements WrapsDriver, WebElement, Coordinates, Locatable { - private static final String[] booleanAttributes = {"async", "autofocus", "autoplay", "checked", "compact", - "complete", "controls", "declare", "defaultchecked", "defaultselected", "defer", "disabled", "draggable", - "ended", "formnovalidate", "hidden", "indeterminate", "iscontenteditable", "ismap", "itemscope", "loop", - "multiple", "muted", "nohref", "noresize", "noshade", "novalidate", "nowrap", "open", "paused", "pubdate", - "readonly", "required", "reversed", "scoped", "seamless", "seeking", "selected", "spellcheck", "truespeed", - "willvalidate"}; - private final HtmlUnitDriver driver_; private final int id_; private final DomElement element_; @@ -255,7 +248,7 @@ void switchFocusToThisIfNeeded() { final boolean jsEnabled = driver_.isJavascriptEnabled(); final boolean oldActiveEqualsCurrent = oldActiveElement.equals(this); try { - final boolean isBody = oldActiveElement.getTagName().toLowerCase().equals("body"); + final boolean isBody = oldActiveElement.getTagName().equalsIgnoreCase("body"); if (jsEnabled && !oldActiveEqualsCurrent && !isBody) { oldActiveElement.element_.blur(); } @@ -285,6 +278,12 @@ public String getAttribute(final String name) { assertElementNotStale(); final String lowerName = name.toLowerCase(); + final String[] booleanAttributes = {"async", "autofocus", "autoplay", "checked", "compact", + "complete", "controls", "declare", "defaultchecked", "defaultselected", "defer", "disabled", "draggable", + "ended", "formnovalidate", "hidden", "indeterminate", "iscontenteditable", "ismap", "itemscope", "loop", + "multiple", "muted", "nohref", "noresize", "noshade", "novalidate", "nowrap", "open", "paused", "pubdate", + "readonly", "required", "reversed", "scoped", "seamless", "seeking", "selected", "spellcheck", "truespeed", + "willvalidate"}; if (element_ instanceof HtmlInput && ("selected".equals(lowerName) || "checked".equals(lowerName))) { return trueOrNull(((HtmlInput) element_).isChecked()); @@ -410,21 +409,11 @@ public String getDomProperty(final String name) { return ScriptRuntime.toCharSequence(ScriptableObject.getProperty(scriptable, lowerName)).toString(); } - // js disabled, fallback to some hacks - if ("disabled".equals(lowerName)) { - if (element_ instanceof DisabledElement) { - return trueOrFalse(((DisabledElement) element_).isDisabled()); - } - } + String element_1 = disabledJsCheck(lowerName); + if (element_1 != null) return element_1; - if ("checked".equals(lowerName)) { - if (element_ instanceof HtmlCheckBoxInput) { - return trueOrFalse(((HtmlCheckBoxInput) element_).isChecked()); - } - else if (element_ instanceof HtmlRadioButtonInput) { - return trueOrFalse(((HtmlRadioButtonInput) element_).isChecked()); - } - } + String element_2 = isElementChecked(lowerName); + if (element_2 != null) return element_2; final String value = element_.getAttribute(lowerName); if (ATTRIBUTE_NOT_DEFINED == value) { @@ -438,6 +427,27 @@ else if (element_ instanceof HtmlRadioButtonInput) { return value; } + private String isElementChecked(String lowerName) { + if ("checked".equals(lowerName)) { + if (element_ instanceof HtmlCheckBoxInput) { + return trueOrFalse(((HtmlCheckBoxInput) element_).isChecked()); + } + else if (element_ instanceof HtmlRadioButtonInput) { + return trueOrFalse(((HtmlRadioButtonInput) element_).isChecked()); + } + } + return null; + } + + private String disabledJsCheck(String lowerName) { + if ("disabled".equals(lowerName)) { + if (element_ instanceof DisabledElement) { + return trueOrFalse(((DisabledElement) element_).isDisabled()); + } + } + return null; + } + @Override public String getDomAttribute(final String name) { assertElementNotStale(); diff --git a/src/test/java/org/openqa/selenium/htmlunit/junit/BrowserRunner.java b/src/test/java/org/openqa/selenium/htmlunit/junit/BrowserRunner.java index ecae580a..b1e55a01 100644 --- a/src/test/java/org/openqa/selenium/htmlunit/junit/BrowserRunner.java +++ b/src/test/java/org/openqa/selenium/htmlunit/junit/BrowserRunner.java @@ -89,7 +89,7 @@ public class BrowserRunner extends Suite { * @throws Throwable If an exception occurs */ public BrowserRunner(final Class klass) throws Throwable { - super(klass, Collections.emptyList()); + super(klass, Collections.emptyList()); if (BrowserVersionClassRunner.containsTestMethods(klass)) { final Set browsers = WebDriverTestCase.getBrowsersProperties();