Illegal exception is thrown on an invalid locator

RESOLVED FIXED in Firefox 46

Status

defect
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: barancev, Assigned: ato)

Tracking

(Blocks 1 bug, {pi-marionette-server})

43 Branch
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/45.0.2454.85 Safari/537.36

Steps to reproduce:

Search by an invalid locator


Actual results:

General webdriver error occurs:

INFO: Executing: findElement [e6e35c7c-0cff-4d9e-96f0-94a4a00cf6f4, findElement {using=css selector, value=#}]
1442239082133	Marionette	DEBUG	conn0 -> {"name":"findElement","parameters":{"using":"css selector","value":"#"},"sessionId":"e6e35c7c-0cff-4d9e-96f0-94a4a00cf6f4"}
1442239082339	Marionette	DEBUG	conn0 client <- {"error":"webdriver error","message":"An invalid or illegal string was specified","stacktrace":null}
Marionette threw an error: SyntaxError: An invalid or illegal string was specified
EM_findElement@chrome://marionette/content/elements.js:620:19
EM_find@chrome://marionette/content/elements.js:485:23
findElementContent/<@chrome://marionette/content/listener.js:1403:1
findElementContent@chrome://marionette/content/listener.js:1402:1
dispatch/</req<@chrome://marionette/content/listener.js:165:22
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
TaskImpl@resource://gre/modules/Task.jsm:275:3
createAsyncFunction/asyncFunction@resource://gre/modules/Task.jsm:249:14
Task_spawn@resource://gre/modules/Task.jsm:164:12
dispatch/<@chrome://marionette/content/listener.js:160:15



Expected results:

Invalid selector error should be returned
http://www.w3.org/TR/webdriver/#handling-errors
http://www.w3.org/TR/webdriver/#element-location-strategies
Assignee

Updated

4 years ago
Assignee: nobody → ato
Blocks: webdriver
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee

Comment 2

4 years ago
Bug 1204504: Wrap invalid CSS query error in invalid selector error

Thanks to Alexei Barantsev for reporting.

r=dburns
Attachment #8662332 - Flags: review?(dburns)
Comment on attachment 8662332 [details]
MozReview Request: Bug 1204504 - Error on invalid selector; r=automatedtester

https://reviewboard.mozilla.org/r/19579/#review17541

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:84
(Diff revision 1)
> +    def test_invalid_css_selector(self):

Please make the name slightly more descriptive.

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:85
(Diff revision 1)
> +        with self.assertRaises(InvalidSelectorException):

This can be done on one line

::: testing/marionette/client/marionette/tests/unit/test_findelement.py:163
(Diff revision 1)
> -        test_html = self.marionette.absolute_url("test.html")
> +        with self.assertRaises(InvalidSelectorException):

This can be done on one line

Update the minor issues and good to go.
Attachment #8662332 - Flags: review?(dburns) → review+
Assignee

Comment 5

3 years ago
Comment on attachment 8662332 [details]
MozReview Request: Bug 1204504 - Error on invalid selector; r=automatedtester

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/19579/diff/1-2/
Attachment #8662332 - Attachment description: MozReview Request: Bug 1204504: Wrap invalid CSS query error in invalid selector error → MozReview Request: Bug 1204504 - Error on invalid selector; r=automatedtester
Assignee

Comment 6

3 years ago
https://reviewboard.mozilla.org/r/19579/#review17541

> This can be done on one line

I find the one line version, where you pass the function reference and its arguments, and unittest will invoke it for you more unreadable because you need to understand what unittest does with it.

Doing it like this is clearer and more understandable, in my opinion, because it removes any doubt what is actually executed.

> This can be done on one line

I don’t particularly like the reflection style API that assertRaises provides to make this a single line.

IMO it adds an additional layer of abstraction that is unnecessary.

Comment 8

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d7025fe68e90
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.