Skip to content

Commit

Permalink
Fixes #1017 - Slick now uses QSA and other selection methods on XHTML…
Browse files Browse the repository at this point in the history 
… documents.
  • Loading branch information
@fabiomcosta
fabiomcosta committed Sep 27, 2010
1 parent 29132ea commit c91a611
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 37 deletions.
10 changes: 10 additions & 0 deletions SlickSpec/assets/JSSpecHelpers.js
View file
Expand Up @@ -3,6 +3,16 @@ String.escapeSingle = String.escapeSingle || function(string){
return (''+string).replace(/(?=[\\\n'])/g,'\\');
};

var isHTML = function(document){
var testNode = document.createElement('div'), isHTML = false;
try {
var id = 'slick_getbyid_test';
testNode.innerHTML = '<a id="'+id+'"></a>';
isHTML = !!document.getElementById(id);

This comment has been minimized.

Sign in to view

Copy link
@subtleGradient

subtleGradient Jan 6, 2011

How does this work without the element being added to the document first?
} catch(e){};
return isHTML;
};

var addEvent = function(obj, event, handler) {
if (obj.addEventListener) {
obj.addEventListener(event, handler, false);
Expand Down
22 changes: 12 additions & 10 deletions SlickSpec/assets/getgetter.js
View file
Expand Up @@ -4,13 +4,15 @@

window.scripts_to_get = window.scripts_to_get || parent.scripts_to_get || window.location.search.match(/\bscript=(.*?\.js)/gi) || [];



for (var i=0, scriptsrc; scriptsrc = scripts_to_get[i]; i++){

This comment has been minimized.

Sign in to view

Copy link
@subtleGradient

subtleGradient Jan 6, 2011

If possible could you try to commit whitespace changes and reformatting as separate commits?
It would be a lot easier to review.
And I have enough trouble reviewing as it is :S
scripts_to_get[i] = scriptsrc = decodeURIComponent(scriptsrc.replace(/^(&?script=)+/,''));

scriptsrc.replace(/^(?!=http|\/)/,'../');


scripts_to_get[i] = scriptsrc = decodeURIComponent(scriptsrc.replace(/^(&?script=)+/, ''));
scriptsrc.replace(/^(?!=http|\/)/, '../');

var written;

if (document.write){
try {
document.write('<scr'+'ipt src="'+ scriptsrc +'" type="text/javascript"><\/script>');
Expand All @@ -20,20 +22,20 @@
}
}

if (!written && document.documentElement.nodeName == 'HTML'){
if (!written && document.documentElement.nodeName.toLowerCase() == 'html'){

This comment has been minimized.

Sign in to view

Copy link
@subtleGradient

subtleGradient Jan 6, 2011

So in XHTML documents it'd be html and not HTML?
Or is XHTML just case sensitive?
var script = document.createElement('script');
script.setAttribute('src', scriptsrc);
script.setAttribute('type', 'text/javascript');
document.documentElement.appendChild(script);
}

// else evalRemote(scriptsrc);

if (/\bbootstrap\b/.test(scriptsrc)) scripts_to_get.splice(i,1);
if (/\bbootstrap\b/.test(scriptsrc)) scripts_to_get.splice(i, 1);

}

this.frameworkName = (scripts_to_get[0] || '').replace(/^.*\//,'');
this.frameworkName = (scripts_to_get[0] || '').replace(/^.*\//, '');

// function evalRemote(url){
// if (!(/^(\/|http:)/).test(url) && /\bmocks\b/.test(document.location.href)) url = '../' + url;
Expand Down
1 change: 1 addition & 0 deletions SlickSpec/mocks/xmlmock1.xml
View file
Expand Up @@ -12,6 +12,7 @@
<camelCasedTag></camelCasedTag>
<camelCasedTag></camelCasedTag>
<camelCasedTag>text</camelCasedTag>
<!-- a xml comment -->
<empty-attr attr=""></empty-attr>
<empty-attr attr="some"></empty-attr>
</HTML>
20 changes: 11 additions & 9 deletions SlickSpec/specs/slick_xml_specs.js
View file
Expand Up @@ -16,15 +16,17 @@ var specsAssetsTemplateXML = function(context){
it('should find '+count+' `'+selector + (!global.cannotDisableQSA ? '` without QSA' : ''), makeSlickTestSearch(selector, count, true));
};

it_should_find(1, 'HTML');
it_should_find(1, '#id_idnode');
it_should_find(1, '[id=id_idnode]');
it_should_find(3, '.class_classNode');
it_should_find(3, '[class=class_classNode]');
it_should_find(0, '[className=class_classNode]');
it_should_find(3, 'camelCasedTag');
it_should_find(1, '#node[style=border]');
it_should_find(1, '[href^=http://]');
it_should_find(14 , '*');

it_should_find(1 , 'HTML');
it_should_find(1 , '#id_idnode');
it_should_find(1 , '[id=id_idnode]');
it_should_find(3 , '.class_classNode');
it_should_find(3 , '[class=class_classNode]');
it_should_find(0 , '[className=class_classNode]');
it_should_find(3 , 'camelCasedTag');
it_should_find(1 , '#node[style=border]');
it_should_find(1 , '[href^=http://]');

it_should_find(1 , ':root');
it_should_find(0 , 'html:root');
Expand Down
5 changes: 3 additions & 2 deletions SlickSpec/specs/specs-engine_bugs.js
View file
Expand Up @@ -3,7 +3,8 @@ var specsSelectorEngineBugs = function(context){
describe('Bugs', function(){
var rootElement;
var testNode;
var isXML = context.isXML(context.document);
var isXML = !isHTML(context.document);//context.isXML(context.document);

var setup = function(){
testNode = context.document.createElement('div');
rootElement = context.document.getElementsByTagName('body')[0];
Expand Down Expand Up @@ -194,7 +195,7 @@ var specsBrowserBugsFixed = function(context){

var rootElement;
var testNode, tmpNode, tmpNode1, tmpNode2, tmpNode3, tmpNode4, tmpNode5, tmpNode6, tmpNode7, tmpNode8, tmpNode9;
var isXML = context.isXML(context.document);
var isXML = !isHTML(context.document);
var results, resultsArray;
var setup = function(){
testNode = context.document.createElement('div');
Expand Down
40 changes: 24 additions & 16 deletions Source/Slick.Finder.js
View file
Expand Up @@ -37,23 +37,31 @@ local.setDocument = function(document){
this.document = document;
var root = this.root = document.documentElement;

// document sort
this.isXMLDocument = this.isXML(document);

this.brokenStarGEBTN
= this.starSelectsClosedQSA
= this.idGetsName
= this.brokenMixedCaseQSA
= this.brokenGEBCN
= this.isHTMLDocument
= false;

var starSelectsClosed, starSelectsComments,
brokenSecondClassNameGEBCN, cachedGetElementsByClassName;

if (!(this.isXMLDocument = this.isXML(document))){
var selected, id;
var testNode = document.createElement('div');
root.appendChild(testNode);

var testNode = document.createElement('div');
this.root.appendChild(testNode);
var selected, id;
// on non-HTML documents innerHTML and getElementsById doesnt work properly
try {
id = 'slick_getbyid_test';
testNode.innerHTML = '<a id="'+id+'"></a>';
this.isHTMLDocument = !!document.getElementById(id);
} catch(e){};

if (this.isHTMLDocument){

// IE returns comment nodes for getElementsByTagName('*') for some documents
testNode.appendChild(document.createComment(''));
Expand All @@ -78,8 +86,8 @@ local.setDocument = function(document){
// IE returns elements with the name instead of just id for getElementById for some documents
try {
id = 'slick_id_gets_name';
testNode.innerHTML = ('<a name='+id+'></a><b id='+id+'></b>');
this.idGetsName = testNode.ownerDocument.getElementById(id) === testNode.firstChild;

This comment has been minimized.

Sign in to view

Copy link
@subtleGradient

subtleGradient Jan 6, 2011

no ownerDocument in XHTML?
testNode.innerHTML = '<a name="'+id+'"></a><b id="'+id+'"></b>';
this.idGetsName = document.getElementById(id) === testNode.firstChild;
} catch(e){};

// Safari 3.2 QSA doesnt work with mixedcase on quirksmode
Expand All @@ -103,11 +111,11 @@ local.setDocument = function(document){

this.brokenGEBCN = cachedGetElementsByClassName || brokenSecondClassNameGEBCN;

this.root.removeChild(testNode);
testNode = null;

}

root.removeChild(testNode);
testNode = null;

// hasAttribute

this.hasAttribute = (root && this.isNativeCode(root.hasAttribute)) ? function(node, attribute) {
Expand Down Expand Up @@ -149,7 +157,7 @@ local.setDocument = function(document){
return aRange.compareBoundaryPoints(Range.START_TO_END, bRange);
} : null ;

this.getUID = (this.isXMLDocument) ? this.getUIDXML : this.getUIDHTML;
this.getUID = (this.isHTMLDocument) ? this.getUIDHTML : this.getUIDXML;

};

Expand Down Expand Up @@ -431,7 +439,7 @@ var combinators = {

var i, item, children;

if (!this.isXMLDocument){
if (this.isHTMLDocument){
getById: if (id){
item = this.document.getElementById(id);
if ((!item && node.all) || (this.idGetsName && item && item.getAttributeNode('id').nodeValue != id)){
Expand Down Expand Up @@ -650,7 +658,7 @@ var pseudos = {
},

'focus': function(node){
return !this.isXMLDocument && this.document.activeElement === node && (node.href || node.type || this.hasAttribute(node, 'tabindex'));
return this.isHTMLDocument && this.document.activeElement === node && (node.href || node.type || this.hasAttribute(node, 'tabindex'));
},

'root': function(node){
Expand Down Expand Up @@ -707,7 +715,7 @@ local.override = function(regexp, method){

local.override(/./, function(expression, found, first){ //querySelectorAll override

if (!this.querySelectorAll || this.nodeType != 9 || local.isXMLDocument || local.brokenMixedCaseQSA || Slick.disableQSA) return false;
if (!this.querySelectorAll || this.nodeType != 9 || !local.isHTMLDocument || local.brokenMixedCaseQSA || Slick.disableQSA) return false;

var nodes, node;
try {
Expand Down Expand Up @@ -758,7 +766,7 @@ local.override(/^[\w-]+$|^\*$/, function(expression, found, first){ // tag overr
/*<class-override>*/

local.override(/^\.[\w-]+$/, function(expression, found, first){ // class override
if (local.isXMLDocument || (!this.getElementsByClassName && this.querySelectorAll)) return false;
if (!local.isHTMLDocument || (!this.getElementsByClassName && this.querySelectorAll)) return false;

var nodes, node, i, hasOthers = !!(found && found.length), className = expression.substring(1);
if (this.getElementsByClassName && !local.brokenGEBCN){
Expand Down Expand Up @@ -786,7 +794,7 @@ local.override(/^\.[\w-]+$/, function(expression, found, first){ // class overri
/*<id-override>*/

local.override(/^#[\w-]+$/, function(expression, found, first){ // ID override
if (local.isXMLDocument || this.nodeType != 9) return false;
if (!local.isHTMLDocument || this.nodeType != 9) return false;

var id = expression.substring(1), el = this.getElementById(id);
if (!el) return found;
Expand Down

2 comments on commit c91a611

@subtleGradient
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks really good.
isHTML is indeed a better thing to know than isXML
since some DOM-like object that doesn't have getByID isn't XML but also isn't HTML. Good Stuff.

Going to have to check it out and test it ofc, but I have no reason to believe that it won't work.

@subtleGradient
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, September. I suck.

Please sign in to comment.