Bug Tracker

Opened 11 years ago

Closed 8 years ago

Last modified 8 years ago

#3834 closed enhancement (wontfix)

Selector syntax errors should give better messages

Reported by: ajpotts Owned by: john
Priority: low Milestone:
Component: selector Version: 1.4.4
Keywords: Cc:
Blocked by: Blocking:

Description

I had a bit of a mangled selector on one of my pages which 1.2.6 simply skipped over, but completely kills 1.3.

I should add that I'm aware with 1.3 the correct response should be "Syntax error, unrecognized expression". But I got a far more impenetrable error.

Full test case here: https://www.partyark.co.uk/html/jquery1.3rc2-bug.html

Change History (9)

comment:1 Changed 11 years ago by john

Milestone: 1.31.3.1
need: ReviewTest Case
Version: 1.3

comment:2 Changed 11 years ago by dmethvin

Keywords: errors added; unrecognized removed
need: Test CasePatch
Summary: Failure on selector syntax errorSelector syntax errors should give better messages
Type: bugenhancement

The error is "a[3] is undefined". As you pointed out, $('.box(:not:has(:radio:checked))') is not valid syntax. jQuery doesn't try to give a nice error message for arbitrary selector syntax errors. I'll recategorize this as a enhancement request for better error handling.

comment:3 Changed 10 years ago by WanderingZombie

I had a lot of problems in IE upgrading from 1.2.6 to 1.3.0 as the following statement did not result in "Syntax error, unrecognized expression: @href=http" being shown when the following statement executed.

$("a[@href=http]").not("a[@href=http://"+location.hostname+"]").each( blah )

Changing

throw "Syntax error, unrecognized expression: " + expr;

to

alert("Syntax error, unrecognized expression: " + expr);

allowed me to see the error and easily fix it by removing the @'s.

But why did I get an uncaught throw error in the first place?

comment:4 Changed 9 years ago by dmethvin

Keywords: needsreview added; selector syntax errors removed
Status: newopen

comment:5 Changed 9 years ago by dmethvin

@WanderingZombie, something was probably catching and discarding the error via try/catch, but not having the code it's impossible to tell.

comment:6 Changed 9 years ago by snover

Current versions of Sizzle don’t throw errors and just return an object with zero elements. Not sure if that is intentional or not.

comment:7 Changed 9 years ago by snover

Milestone: 1.3.1
Priority: minorlow
Version: 1.31.4.4

comment:8 Changed 8 years ago by dmethvin

Resolution: wontfix
Status: openclosed

As we move towards using the browser's native querySelectorAll implementations, there won't be a lot we can do to improve error messages in Sizzle. It might be possible to do better error checking via a plugin but I don't think we'd put it in the production code.

comment:9 Changed 8 years ago by dmethvin

Keywords: needsreview removed
Note: See TracTickets for help on using tickets.