From 430d26eab65b7fcd344c150dd9b9ae64a397ecc9 Mon Sep 17 00:00:00 2001
From: Dave Roberts
Date: Sat, 17 Aug 2024 12:01:51 -0500
Subject: [PATCH] Issue #559 (#627)
* Add query-from and query-all-from
* Deprecate child and children
* Make get-active-element a public API
Previously, get-active-element* was a private API. This simply changes
the name to remove the "*" and makes it public.
* Update doc strings and user guide to deprecate :active
Suggests to use get-active-element instead of (query driver :active).
* Modify get-active-element to use unwrap-webdriver-object
* Update CHANGELOG to better reflect issue 559 changes
* Fix redundant let found by clj-kondo
* Suppress clj-kondo warnings for calling deprecated child/children
* Suppress Eastwood warnings for calling deprecated child/children
Test suite still calls child/children to detect regressions in these
deprecated, but not yet removed, functions.
* Credit dgr in CHANGELOG
* Clarify issues with :active in User Guide and query doc string
---
.clj-kondo/config.edn | 5 ++-
CHANGELOG.adoc | 7 +++
deps.edn | 3 +-
doc/01-user-guide.adoc | 30 ++++++++-----
env/test/resources/static/test.html | 21 +++++++++
src/etaoin/api.clj | 70 +++++++++++++++++++++++++----
test/etaoin/api_test.clj | 61 ++++++++++++++++++++++---
7 files changed, 171 insertions(+), 26 deletions(-)
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index bbd0f54..6aae867 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -8,4 +8,7 @@
;; finer grained error reporting
{:macroexpand
{etaoin.impl.util/defmethods etaoin.impl.util/defmethods
- etaoin.impl.util/with-tmp-file etaoin.impl.util/with-tmp-file}}}
+ etaoin.impl.util/with-tmp-file etaoin.impl.util/with-tmp-file}}
+ :linters {:deprecated-var
+ {:exclude {etaoin.api/child {:namespaces [etaoin.api-test]}
+ etaoin.api/children {:namespaces [etaoin.api-test]}}}}}
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
index ec03248..ff7b78d 100644
--- a/CHANGELOG.adoc
+++ b/CHANGELOG.adoc
@@ -19,6 +19,13 @@ A release with an intentional breaking changes is marked with:
// (adjust these in publish.clj as you see fit)
== Unreleased
+* Changes
+** {issue}559[#559]: Create `query-from` and `query-all-from` as replacements for `child` and `children`. Rewrite logic such that `q` parameter allows for vector syntax similar to `query` and `query-all`. Deprecate `child` and `children`. ({person}dgr[@dgr])
+** {issue}559[#559]: Make `get-active-element` a public API. This was previously private. ({person}dgr[@dgr])
+
+* Docs
+** {issue}559[#559]: Deprecate use of `:active` with `query` and other APIs that use `query` under the hood. ({person}dgr[@dgr])
+
== v1.1.41 [minor breaking] - 2024-08-14 [[v1.1.41]]
* Minor breaking
diff --git a/deps.edn b/deps.edn
index 08e6bdf..55478a1 100644
--- a/deps.edn
+++ b/deps.edn
@@ -69,7 +69,8 @@
:main-opts ["-m" "eastwood.lint" {:source-paths ["src"]
:test-paths ["test"]
:add-linters [:performance]
- :exclude-linters [:local-shadows-var]}]}
+ :exclude-linters [:local-shadows-var]
+ :ignored-faults {:deprecations {etaoin.api-test true}}}]}
:build {:deps {io.github.clojure/tools.build {:mvn/version "0.10.5"}}
:extra-paths ["build"]
diff --git a/doc/01-user-guide.adoc b/doc/01-user-guide.adoc
index e231631..97ac34d 100644
--- a/doc/01-user-guide.adoc
+++ b/doc/01-user-guide.adoc
@@ -291,13 +291,13 @@ endif::[]
;; Ok, now let's try something trickier
;; Maybe we are interested in what value the infobox holds for the Family row:
(let [wikitable (e/query driver {:css "table.infobox.vevent tbody"})
- row-els (e/children driver wikitable {:tag :tr})]
+ row-els (e/query-all-from driver wikitable {:tag :tr})]
(for [row row-els
:let [header-col-text (e/with-http-error
(e/get-element-text-el driver
- (e/child driver row {:tag :th})))]
+ (e/query-from driver row {:tag :th})))]
:when (= "Family" header-col-text)]
- (e/get-element-text-el driver (e/child driver row {:tag :td}))))
+ (e/get-element-text-el driver (e/query-from driver row {:tag :td}))))
;; => ("Lisp")
;; Etaoin gives you many options; we can do the same-ish in one swoop in XPath:
@@ -499,17 +499,27 @@ Use `exists?` to check for element existence:
:xpath-sel: https://www.w3schools.com/xml/xpath_syntax.asp
:css-sel: https://www.w3schools.com/cssref/css_selectors.asp
-* `:active` finds the current active element.
-The Google page, for example, automatically places the focus on the search input.
-So there is no need to click on it first:
+* `:active` finds the currently active element.
+Note that querying for `:active` is deprecated.
+Users are encouraged to call `get-active-element` to retrieve the active element.
+The Google homepage, for example, automatically places the focus on the search input, so there is no need to click on it first.
+
[source,clojure]
----
+;; Deprecated
(e/go driver "https://google.com")
(e/fill driver :active "Let's search for something" k/enter)
+;; Better
+(e/go driver "https://google.com")
+(e/fill-el driver (e/get-active-element driver) "Let's search for something" k/enter)
+;; Or best... This case is so common that the API includes `fill-active`
+(e/go driver "https://google.com")
+(e/fill-active driver "Let's search for something" k/enter)
----
-* any other keyword is translated to an html id attribute:
+* any other keyword is translated to an HTML ID attribute.
+Note that you cannot query for an HTML element with an ID of "active" using this keyword syntax because the special keyword `:active` conflicts.
+If you want to query for an element with an ID of "active", use the map syntax, described below (e.g., `{:id "active"}` or `{:id :active}`).
+
[source,clojure]
----
@@ -1156,7 +1166,7 @@ A quick example of entering ordinary characters while holding Shift:
(e/refresh driver)
(e/wait 1) ;; maybe we need a sec for active element to focus
(e/fill-active driver (k/with-shift "caps is great"))
-(e/get-element-value driver :active)
+(e/get-element-value-el driver (e/get-active-element driver))
;; => "CAPS IS GREAT"
----
@@ -1168,7 +1178,7 @@ Let's duplicate the text via select-all, copy, and paste keyboard shortcuts:
(if (= "Mac OS X" (System/getProperty "os.name"))
(e/fill-active driver (k/with-command "a") (k/with-command "c") k/arrow-right " " (k/with-command "v"))
(e/fill-active driver (k/with-ctrl "a") (k/with-ctrl "c") k/arrow-right " " (k/with-ctrl "v")))
-(e/get-element-value driver :active)
+(e/get-element-value-el driver (e/get-active-element driver))
;; => "CAPS IS GREAT CAPS IS GREAT"
----
@@ -1180,7 +1190,7 @@ And now let's clear the input by:
[source,clojure]
----
(e/fill-active driver k/home (k/with-shift k/end) k/delete)
-(e/get-element-value driver :active)
+(e/get-element-value-el driver (e/get-active-element driver))
;; => ""
----
diff --git a/env/test/resources/static/test.html b/env/test/resources/static/test.html
index a310cf8..89554cd 100644
--- a/env/test/resources/static/test.html
+++ b/env/test/resources/static/test.html
@@ -248,6 +248,27 @@ Find text with quote
+
+
+ We want to skip over this
+
+
+
+
+
Shadow DOM
I'm not in the shadow DOM
diff --git a/src/etaoin/api.clj b/src/etaoin/api.clj
index 72c5aaf..581deff 100644
--- a/src/etaoin/api.clj
+++ b/src/etaoin/api.clj
@@ -23,7 +23,8 @@
- [[get-status]] [[create-session]] [[delete-session]]
**Querying/Selecting DOM Elements**
- - [[query]] [[query-all]] [[query-tree]]
+ - [[query]] [[query-all]] [[query-tree]] [[query-from]] [[query-all-from]]
+ - [[get-active-element]]
- [[query-from-shadow-root]] [[query-from-shadow-root-el]] [[query-all-from-shadow-root]] [[query-all-from-shadow-root-el]]
- [[has-shadow-root?]] [[has-shadow-root-el?]]
- [[exists?]] [[absent?]]
@@ -314,7 +315,7 @@
;; active element
;;
-(defn- get-active-element*
+(defn get-active-element
"Have `driver` return the active element on the page.
An active element is the one with the current focus.
@@ -325,7 +326,8 @@
(-> (execute {:driver driver
:method :get
:path [:session (:session driver) :element :active]})
- :value first second))
+ :value
+ (unwrap-webdriver-object web-element-identifier)))
;;
;; windows
@@ -595,10 +597,14 @@
Query `q` can be:
- - `:active` the current active element
+ - `:active` the current active element. Note that this is deprecated.
+ Use [[get-active-element]] instead to find the currently active element.
- a keyword to find element by it's ID attribute:
- `:my-id`
- (use `{:id \"my-id\"}` for ids that cannot be represented as keywords)
+ - Note that `:active` conflicts with this usage and therefore you
+ cannot search for a keyword named `:active` and expect to find an element
+ with ID equal to \"active\". In this case, use `{:id \"active\"}`.
- an XPath expression:
- `\".//input[@id='uname']\"`
- a map with either `:xpath` or `:css`:
@@ -623,7 +629,7 @@
(cond
(= q :active)
- (get-active-element* driver)
+ (get-active-element driver)
(vector? q)
(apply query driver q)
@@ -683,28 +689,76 @@
(find-elements* driver loc term))
qs))
-(defn child
+(defn ^{:deprecated "1.1.42"} child
"Uses `driver` to return single element satisfying query `q` under given `ancestor-el` element.
See [[query]] for details on `q`.
+ NOTE: `child` has been deprecated in favor of `query-from` which
+ more closely supports `query`'s syntax for `q` and has a name which
+ better matches its functionality and the latest W3C WebDriver spec.
+
https://www.w3.org/TR/webdriver2/#dfn-find-element-from-element"
[driver ancestor-el q]
{:pre [(some? ancestor-el)]}
(let [[loc term] (query/expand driver q)]
(find-element-from* driver ancestor-el loc term)))
-(defn children
+(defn ^{:deprecated "1.1.42"} children
"Use `driver` to return a vector of unique elements satisfying query `q` under given `ancestor-el` element.
See [[query]] for details on `q`.
+ NOTE: `children` has been deprecated in favor of `query-all-from` which
+ more closely supports `query`'s syntax for `q` and has a name which
+ better matches its functionality and the latest W3C WebDriver spec.
+
https://www.w3.org/TR/webdriver2/#find-elements-from-element"
[driver ancestor-el q]
{:pre [(some? ancestor-el)]}
(let [[loc term] (query/expand driver q)]
(find-elements-from* driver ancestor-el loc term)))
+(defn query-from
+ "Use `driver` to return a single element satisfying query `q`,
+ starting the search at the element specified by `el`. `query-from`
+ is similar to `query` but starts the search from `el` rather than
+ the DOM root.
+
+ See [[query]] for details on `q`.
+
+ https://www.w3.org/TR/webdriver2/#dfn-find-element-from-element"
+ [driver el q]
+ (if (sequential? q)
+ (follow-path-from-element* driver el q)
+ (let [[loc term] (query/expand driver q)]
+ (find-element-from* driver el loc term))))
+
+(defn query-all-from
+ "Use `driver` to return a vector of elements satisfying query `q`,
+ starting the search at the element specified by `el`. If `q` is a
+ vector of queries, then the search starts from `el` and identifies
+ single candidates for the first item in `q`, and then uses that
+ element as the root of the next search, with the exception of the
+ last item, which is then searched for all matching
+ elements. `query-all-from` is similar to `query-all` but starts the
+ search from `el` rather than the DOM root.
+
+ See [[query]] for details on `q`.
+
+ https://www.w3.org/TR/webdriver2/#dfn-find-elements-from-element"
+ [driver el q]
+ (if (sequential? q)
+ (let [last-q (last q)
+ but-last-q (butlast q)
+ but-last-el (if (some? but-last-q)
+ (follow-path-from-element* driver el but-last-q)
+ el)
+ [loc term] (query/expand driver last-q)]
+ (find-elements-from* driver but-last-el loc term))
+ (let [[loc term] (query/expand driver q)]
+ (find-elements-from* driver el loc term))))
+
;; actions
(declare el->ref wait)
@@ -2659,7 +2713,7 @@
(defn fill-active
"Have `driver` fill active element with `text` (and optionally `more` text)."
[driver text & more]
- (let [el (get-active-element* driver)]
+ (let [el (get-active-element driver)]
(apply fill-el driver el text more)))
(defn fill
diff --git a/test/etaoin/api_test.clj b/test/etaoin/api_test.clj
index 67a9f5a..719a6f7 100644
--- a/test/etaoin/api_test.clj
+++ b/test/etaoin/api_test.clj
@@ -595,12 +595,17 @@
(is (not= moved-rect maximed-rect)))))))
(deftest test-active-element
- (e/when-not-safari *driver*
- (doto *driver*
- (e/click {:id :set-active-el})
- (-> (e/get-element-attr :active :id)
- (= "active-el-input")
- is))))
+ (e/click *driver* {:id :set-active-el})
+ ;; Test old query API since get-element-attr uses query underneath
+ (is (= "active-el-input" (e/get-element-attr *driver* :active :id)))
+ ;; Test old query using :active directly
+ (is (= "active-el-input" (e/get-element-attr-el *driver*
+ (e/query *driver* :active)
+ :id)))
+ ;; Test new get-active-element API. This is preferred.
+ (is (= "active-el-input" (e/get-element-attr-el *driver*
+ (e/get-active-element *driver*)
+ :id))))
(deftest test-element-text
(let [text (e/get-element-text *driver* {:id :element-text})]
@@ -796,6 +801,50 @@
(is (str/includes? (first children-texts) "From the depths I've come!"))
(is (str/includes? (last children-texts) "I've come from the darkness of the pit!"))))
+(deftest test-query-from
+ (testing "single item query"
+ (let [parent-el (e/query *driver* {:css "#wc3-barks"})
+ child-el (e/query-from *driver* parent-el {:css ".crypt-lord"})
+ tag-name (str/lower-case (e/get-element-tag-el *driver* child-el))
+ tag-text (e/get-element-text-el *driver* child-el)]
+ (is (= "span" tag-name))
+ (is (str/includes? tag-text "From the depths I've come!"))))
+ (testing "vector query"
+ (let [start-el (e/query *driver* :deep-query-root)
+ final-el (e/query-from *driver*
+ start-el
+ [{:class "intermediate-node-1-2"}
+ ;; skip over intermediate-node-2
+ {:css "#intermediate-node-3"}
+ :intermediate-node-4
+ {:tag "li"}])
+ tag-name (str/lower-case (e/get-element-tag-el *driver* final-el))
+ tag-text (e/get-element-text-el *driver* final-el)]
+ (is (= "li" tag-name))
+ (is (= "One" tag-text)))))
+
+(deftest test-query-all-from
+ (testing "single item query"
+ (let [parent-el (e/query *driver* {:css "#wc3-barks"})
+ children-els (e/query-all-from *driver* parent-el {:css "p"})
+ children-texts (map #(e/get-element-text-el *driver* %) children-els)]
+ (is (= ["p" "p"] (map #(str/lower-case (e/get-element-tag-el *driver* %)) children-els)))
+ (is (str/includes? (first children-texts) "From the depths I've come!"))
+ (is (str/includes? (last children-texts) "I've come from the darkness of the pit!"))))
+ (testing "vector query"
+ (let [start-el (e/query *driver* :deep-query-root)
+ final-els (e/query-all-from *driver*
+ start-el
+ [{:class "intermediate-node-1-2"}
+ ;; skip over intermediate-node-2
+ {:css "#intermediate-node-3"}
+ :intermediate-node-4
+ {:tag "li"}])
+ tag-names (mapv #(str/lower-case (e/get-element-tag-el *driver* %)) final-els)
+ tag-texts (mapv #(e/get-element-text-el *driver* %) final-els)]
+ (is (= ["li" "li" "li"] tag-names))
+ (is (= ["One" "Two" "Three"] tag-texts)))))
+
(deftest test-postmortem
(let [dir-tmp (format
"%s/%s"