#14029 closed bug (notabug)
japanese id and class selector is very slow
Reported by: | Owned by: | ||
---|---|---|---|
Priority: | undecided | Milestone: | None |
Component: | unfiled | Version: | 1.10.1 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
There are a bug in regular expression rquickExpr which cause japanese id, class not be recognized correctly so not get optimized with getElementById and getElementsByClassName.
Normal(OK) : $("#a") is translated to $(document.getElementById("a") Expect : $("#あ") will be translated to $(document.getElementById("あ") Result(NG) : $("#あ") is translated to $(document.querySelectorAll("#あ")
The same with class name. Normal(OK): $(".a") is translated to $(document.getElementsByClassName("a") Expect : $(".あ") will be translated to $(document.getElementsByClassName("あ") Result(NG) : $(".あ") is translated to $(document.querySelectorAll(".あ")
I changed 3 "[\w-]" in rquickExpr to "[\s.,+>~:\[]" so fixed the problem. line 68:
rquickExpr = /(?:\s*(<[\w\W]+>)[>]*|#([\w-]*))$/,
ー>
rquickExpr = /(?:\s*(<[\w\W]+>)[>]*|#([\s.,+>~:\[]*))$/,
line 969:
Easily-parseable/retrievable ID or TAG or CLASS selectors rquickExpr = /(?:#([\w-]+)|(\w+)|\.([\w-]+))$/,
ー>
rquickExpr = /(?:#([\s.,+>~:\[]+)|(\w+)|\.([\s.,+>~:\[]+))$/,
I hope you commit this modification.
Change History (5)
comment:1 follow-up: 2 Changed 10 years ago by
Owner: | set to [email protected]… |
---|---|
Status: | new → pending |
comment:2 Changed 10 years ago by
Replying to dmethvin:
Can you quantify "very slow" with a jsperf.com test? querySelectorAll isn't that slow.
I did test: http://jsperf.com/non-english-id-jquery-vs-english-id-jquery
rquickExpr
isn't meant to catch all possible valid IDs, only those that can be easily recognized without the risk of misinterpreting escapes for example. I'm not sure if that change opens up any issues or not, but having the jsperf would be a good start to see what impact it has.
I understand. Please confirm it carefully.
comment:3 Changed 10 years ago by
Owner: | [email protected]… deleted |
---|---|
Status: | pending → new |
comment:4 Changed 10 years ago by
Resolution: | → notabug |
---|---|
Status: | new → closed |
It is slower, but it's not very slow. It takes the QSA path, which is what any selector not matching rquickExpr would attempt next. We could open up rquickExpr to all valid CSS identifiers, but we could see the quick path slow down for all selectors. Also, I don't see the need. If anyone disagrees, feel free to reopen.
comment:5 Changed 10 years ago by
in jquery 2, it is more slower in contrast to english id. But i can not include jquery 2 in jsperf.
Can you quantify "very slow" with a jsperf.com test? querySelectorAll isn't that slow.
rquickExpr
isn't meant to catch all possible valid IDs, only those that can be easily recognized without the risk of misinterpreting escapes for example. I'm not sure if that change opens up any issues or not, but having the jsperf would be a good start to see what impact it has.