Opened 11 years ago

Closed 7 years ago

Last modified 6 years ago

#7479 closed defect (fixed)

[patch][cla] dojo.query breaks for expressions like "a[href*='foo=bar']"

Reported by: ptwobrussell Owned by: dylan
Priority: low Milestone: 1.7.5
Component: Query Version: 1.1.1
Keywords: Cc:
Blocked By: Blocking:

Description

The specific error you'll get is

failure in exprssion: .//A[@href='foo='bar'']

Found this out the hard way when I was trying to use dojo.query to find a hyperlink on a page that had a href= with an equals sign in it.

None of the tests for query seemed to cover the case when you have a selector with a substring in it that contains an equals sign, so that's probably why it slipped through.

The specific issue is that the expression in the state machine on line 152 (r14546) is matching on the = sign when it shouldn't be.

I tried to generalize a good solution to this one, but admittedly haven't tested the solution extensively. It doesn't break any tests, however, and does solve this problem, so maybe it'll work. The thinking was that the simplest way to get around this edge case might be to treat it similar to the way that lexers optimize out comments in source code. i.e. ignore anything between single quotes completely since it can't be part of an actual CSS expression and should be treated as a constant of sorts.

Index: query.js
===================================================================
--- query.js	(revision 14916)
+++ query.js	(working copy)
@@ -70,6 +70,7 @@
 		var inClass = -1;
 		var inId = -1;
 		var inTag = -1;
+		var inSingleQuote = false;
 		var lc = ""; // the last character
 		var cc = ""; // the current character
 		var pStart;
@@ -122,6 +123,15 @@
 				inTag = x;
 			}
 
+            if (cc == "'" && inSingleQuote) {
+               inSingleQuote = false;
+                continue;
+            }
+            else if (cc == "'" || inSingleQuote) {
+                inSingleQuote = true;
+                continue;
+            }
+
 			if(inBrackets >= 0){
 				// look for a the close first
 				if(cc == "]"){

Cheers.

Change History (15)

comment:1 Changed 11 years ago by dylan

Milestone: tbd1.2
Owner: changed from anonymous to dante
Summary: dojo.query breaks for expressions like "a[href*='foo=bar']"[patch][cla] dojo.query breaks for expressions like "a[href*='foo=bar']"

comment:2 Changed 11 years ago by dante

see also #7512

comment:3 Changed 11 years ago by bill

Milestone: 1.21.3

as per today's meeting, punting these core bugs

comment:4 Changed 10 years ago by dante

Milestone: 1.3future
Owner: changed from dante to alex

please review and address. patch is likely stale since recent refactor, but willing to bet issue exists. can work up series of unit tests for oddball query bugs like this, if preferred.

comment:5 Changed 10 years ago by bill

Component: CoreQuery

comment:6 Changed 8 years ago by Chris Mitchell

Owner: changed from alex to dylan

please review/triage

comment:7 Changed 7 years ago by ben hockey

Keywords: needsreview added
Priority: highlow

comment:8 Changed 7 years ago by ben hockey

#8631 is a duplicate of this ticket.

comment:9 Changed 7 years ago by ben hockey

#9193 is a duplicate of this ticket.

comment:10 Changed 7 years ago by bill

In [27955]:

test case for attributes with equals signs, refs #7479.

comment:11 Changed 7 years ago by bill

Keywords: needsreview removed
Milestone: future1.3
Resolution: fixed
Status: newclosed

comment:12 Changed 7 years ago by bill

Milestone: 1.31.8
Resolution: fixed
Status: closedreopened

Oh, OK the bug is still there but it only triggers in IE. I will apply a modified version of your patch, thanks!

comment:13 Changed 7 years ago by bill

Resolution: fixed
Status: reopenedclosed

In [27969]:

Fix for handled quoted values in IE branch of code, fixes #7479, #13084 on IE !strict. Thanks to ptwobrussell for basis of patch.

comment:14 Changed 6 years ago by Colin Snover

In [30042]:

Fix for handled quoted values in IE branch of code, fixes #7479, #13084 on IE !strict. Thanks to ptwobrussell for basis of patch. Backport to 1.7

comment:15 Changed 6 years ago by Colin Snover

Milestone: 1.81.7.5
Note: See TracTickets for help on using tickets.