Opened 5 years ago

Closed 5 years ago

#18508 closed enhancement (duplicate)

if-then-else by throwing exceptions in selector/acme.js

Reported by: wpausch Owned by: Kris Zyp
Priority: undecided Milestone: tbd
Component: Query Version: 1.10.4
Keywords: Cc:
Blocked By: Blocking:

Description

In selector/acme.js, in the code block pasted below throwing the exception is apparently equivalent to having an ordinary if-then-else block.

My problem with that piece of code is that it is frequently executed in the context of dgrid, and interferes with the "stop on exceptions" feature of e.g. Chrome.

In the end I e.g. have to make the Debugger continue five times just because I put focus into a Editor inside the dgrid (or something similar). This is a bit annoying while debugging.

Thus I advocate for replacing the "throw """ by "return getQueryFunc(...)" that will be executed after the exception has been caught just below.

			return _queryFuncCacheQSA[query] = function(root){
				try{
					// the QSA system contains an egregious spec bug which
					// limits us, effectively, to only running QSA queries over
					// entire documents.  See:
					//		http://ejohn.org/blog/thoughts-on-queryselectorall/
					//	despite this, we can also handle QSA runs on simple
					//	selectors, but we don't want detection to be expensive
					//	so we're just checking for the presence of a space char
					//	right now. Not elegant, but it's cheaper than running
					//	the query parser when we might not need to
					if(!((9 == root.nodeType) || nospace)){ throw ""; }
					var r = root[qsa](tq);
					// skip expensive duplication checks and just wrap in a NodeList
					r[noZip] = true;
					return r;
				}catch(e){
					// else run the DOM branch on this query, ensuring that we
					// default that way in the future
					return getQueryFunc(query, true)(root);
				}
			};

Change History (2)

comment:1 Changed 5 years ago by bill

Component: GeneralQuery
Owner: set to Kris Zyp

I agree, the current code is bizarre.

I'm unclear if this code will ever throw an exception outside of the if() statement though. But the automated tests should tell us.

comment:2 Changed 5 years ago by bill

Resolution: duplicate
Status: newclosed

Duplicate of #17515.

Note: See TracTickets for help on using tickets.