Skip to content

Commit

Permalink
* Fixing document.id and Element.getElementById to handle special cha…
Browse files Browse the repository at this point in the history 
…rs. [#1013]
  • Loading branch information
@cpojer
cpojer committed Sep 21, 2010
1 parent 5c6c92c commit 19e65d4
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions Source/Element/Element.js
View file
Expand Up @@ -219,7 +219,7 @@ Document.implement({
var types = {

string: function(id, nocash, doc){
id = Slick.find(doc, '#' + id);
id = Slick.find(doc, '#' + id.replace(/(\W)/g, '\\$1'));

This comment has been minimized.

Sign in to view

Copy link
@fabiomcosta

fabiomcosta Sep 22, 2010

Member

I was thinking, we could use Element.getElementById(doc, id) here cant we?

This comment has been minimized.

Sign in to view

Copy link
@cpojer

cpojer Sep 22, 2010

Author Member

Not really.

This comment has been minimized.

Sign in to view

Copy link
@istrasoft

istrasoft Apr 26, 2012

Somehow the presence of a space in the id is handled correctly in all browsers (even though it is agaist W3C spec) in standards mode and all but IE in quirks mode. The reason is that once switched to Quirks mode (or IE < 8), document.querySelectorAll is no longer available and Slick.parse is called instead. the huge regexp used for parsing is following W3C standards and does not support spaces in IDs, which document.getElementById and document.querySelectorAll do for some reason, even in standards mode.

By the way, I wonder if we ignore issues fixed by the use of Slick, isn't the new version of document.id slower than the previous document.getElementById ?

Also, in local.search, as soon as the ID has any \W character like $ for example, expression.match(reSimpleSelector) fails even if the selector is still a simple one. Couldn't the reSimpleSelector regexp be adapted for such simple cases ?
return (id) ? types.element(id, nocash) : null;
},

Expand Down Expand Up @@ -606,7 +606,7 @@ Element.implement({
},

getElementById: function(id){
return document.id(Slick.find(this, '#' + id));
return document.id(Slick.find(this, '#' + ('' + id).replace(/(\W)/g, '\\$1')));

This comment has been minimized.

Sign in to view

Copy link
@digitarald

digitarald Sep 22, 2010

Contributor

Why does getElementById also replace characters when document.id handles the same pattern? Don't we need the replace in only one place?

This comment has been minimized.

Sign in to view

Copy link
@cpojer

cpojer Sep 22, 2010

Author Member

Slick.find(this) != Slick.find(doc).
},

getSelected: function(){
Expand Down
2 changes: 1 addition & 1 deletion Specs
Submodule Specs updated 1 files
+33 −0 1.3client/Element/Element.js

0 comments on commit 19e65d4

Please sign in to comment.