Last Comment Bug 799008 - findElements does not scope to parent
: findElements does not scope to parent
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: Marionette (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla19
Assigned To: Alfredos-Panagiotis Damkalis [:fredy]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-07 22:10 PDT by James Lal [:lightsofapollo] (inactive)
Modified: 2012-11-02 13:36 PDT (History)
4 users (show)
ryanvm: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
bugfix (751 bytes, patch)
2012-10-08 07:33 PDT, Alfredos-Panagiotis Damkalis [:fredy]
no flags Details | Diff | Review
fix and unit test (2.58 KB, patch)
2012-10-16 14:20 PDT, Alfredos-Panagiotis Damkalis [:fredy]
no flags Details | Diff | Review
patch v2 (2.54 KB, patch)
2012-10-17 15:10 PDT, Alfredos-Panagiotis Damkalis [:fredy]
dburns: review+
Details | Diff | Review

Description James Lal [:lightsofapollo] (inactive) 2012-10-07 22:10:26 PDT
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] (inactive) 2012-10-07 22:11:19 PDT
the workaround is to do something like

marionette.find_elements('css selector', '#article-head div');
Comment 2 Alfredos-Panagiotis Damkalis [:fredy] 2012-10-08 07:33:35 PDT
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 2012-10-08 08:32:00 PDT
Comment on attachment 669143 [details] [diff] [review]
bugfix

Can you add a test case for this too?
Comment 4 Alfredos-Panagiotis Damkalis [:fredy] 2012-10-16 14:20:20 PDT
Created attachment 672024 [details] [diff] [review]
fix and unit test
Comment 5 David Burns :automatedtester 2012-10-16 15:01:12 PDT
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] 2012-10-16 21:12:27 PDT
(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] 2012-10-16 21:14:04 PDT
(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] 2012-10-17 15:10:41 PDT
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 2012-10-18 06:57:30 PDT
Comment on attachment 672541 [details] [diff] [review]
patch v2

awesome work!
Comment 10 David Burns :automatedtester 2012-10-18 07:12:08 PDT
landed in m-i https://hg.mozilla.org/integration/mozilla-inbound/rev/f5760fcd8420
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-10-18 18:41:35 PDT
https://hg.mozilla.org/mozilla-central/rev/f5760fcd8420
Comment 12 Jonathan Griffin (:jgriffin) 2012-10-31 16:58:37 PDT
https://hg.mozilla.org/projects/ash/rev/059cdea17d24
Comment 13 Jonathan Griffin (:jgriffin) 2012-11-02 13:36:57 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/4ac2d634671d

Note You need to log in before you can comment on or make changes to this bug.