799008 - findElements does not scope to parent

Status: RESOLVED FIXED

Description

[James Lal [:lightsofapollo]]

STR:

marionette.navigate('https://developer.mozilla.org/en-US/docs/Mozilla/Boot_to_Gecko/Marionette_for_Interactive_Python')

head = marionette.find_element('css selector', '#article-head')
head.find_elements('css selector', 'div')

expected:

find elements to return the single 'div' element inside #article-head

actual:

all div elements in the entire document are returned.

--

My best guess is this related to: https://mxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-elements.js#251

My build is from October 2nd (which is far after this code has been modified).

Comment 1

[James Lal [:lightsofapollo]]

the workaround is to do something like

marionette.find_elements('css selector', '#article-head div');

Comment 2

[Alfredos-Panagiotis Damkalis [:fredy]]

Created attachment 669143 [details] [diff] [review]
bugfix

At findElements function, on css-selector case, searching for the elements always starts at the rootNode which is the document and not at the current node (startNode).

The attached patch corrects this behavior.

Comment 3

[David Burns :automatedtester]

Comment on attachment 669143 [details] [diff] [review]
bugfix

Can you add a test case for this too?

Comment 4

[Alfredos-Panagiotis Damkalis [:fredy]]

Created attachment 672024 [details] [diff] [review]
fix and unit test

Comment 5

[David Burns :automatedtester]

Comment on attachment 672024 [details] [diff] [review]
fix and unit test


> 
>+    def test_selector_scope(self):

Can we make the test name more verbose so we can, at a glance, work out what it is supposed to do. E.g. test_css_selector_scope_doesnt_start_at_rootnode

>+        el = self.marionette.execute_script("return window.document.getElementById('mozLink');")
>+        nav_el = self.marionette.execute_script("return window.document.getElementById('testDiv');")

Why are you using execute script instead of find_element here?

Comment 6

[Alfredos-Panagiotis Damkalis [:fredy]]

(In reply to David Burns :automatedtester from comment #5)
> Comment on attachment 672024 [details] [diff] [review]
> fix and unit test
> 
> 
> > 
> >+    def test_selector_scope(self):
> 
> Can we make the test name more verbose so we can, at a glance, work out what
> it is supposed to do. E.g. test_css_selector_scope_doesnt_start_at_rootnode
> 
Yes, that's sound better. I am going to fix that. 

> >+        el = self.marionette.execute_script("return window.document.getElementById('mozLink');")
> >+        nav_el = self.marionette.execute_script("return window.document.getElementById('testDiv');")
> 
> Why are you using execute script instead of find_element here?

I just saw it on some other tests in the same file, e.g. test_xpath.
I thought also that, just in case, the find_element function don't work, using execute script will make this test pass.
I can also fix this behavior.

Comment 7

[Alfredos-Panagiotis Damkalis [:fredy]]

(In reply to Alfredos-Panagiotis Damkalis (irc: fredy) from comment #6)
> Yes, that's sound better. I am going to fix that. 
> 
*that sounds better

Comment 8

[Alfredos-Panagiotis Damkalis [:fredy]]

Created attachment 672541 [details] [diff] [review]
patch v2

Name of the test is fixed and the execute_scripts replaced with find_element.

Comment 9

[David Burns :automatedtester]

Comment on attachment 672541 [details] [diff] [review]
patch v2

awesome work!

Comment 10

[David Burns :automatedtester]

landed in m-i https://hg.mozilla.org/integration/mozilla-inbound/rev/f5760fcd8420

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f5760fcd8420

Comment 12

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/projects/ash/rev/059cdea17d24

Comment 13

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4ac2d634671d