fgnass / domino

Server-side DOM implementation based on Mozilla's dom.js
BSD 2-Clause "Simplified" License
768 stars 120 forks source link

Using id in querySelectorAll returns a non empty object #99

Closed jdlrobson closed 7 years ago

jdlrobson commented 7 years ago

When using querySelectorAll (on an id selector) I get some unexpected behavior: var window = domino.createWindow('<!DOCTYPE html><html><body></body></html>'); const match = window.document.querySelectorAll('#coordinates'); assert.ok(match.length === 0); assert.ok(Array.from(match).length == 0);

When inspecting match[0] it is undefined.

When the selector is changed to #coordinates,span it works as expected.

Is this a bug?

cscott commented 7 years ago

I'm not quite sure I understand what the unexpected behavior is. In JavaScript, accessing an element past the end of an array always returns undefined. Perhaps it is the "as expected" behavior which is at error?

Sanity-checking in a browser console in Chrome:

> newDoc = document.implementation.createHTMLDocument();
< #document
> newDoc.documentElement.outerHTML
< "<html><head></head><body></body></html>"
> match = newDoc.querySelectorAll('#coordinates');
< []
> match.length
< 0
> match[0]
< undefined
> Array.from(match).length
< 0
cscott commented 7 years ago

Oh, nevermind, I see:

$ node
> domino = require('./');
> var domimpl = domino.createDOMImplementation();
> var doc = domimpl.createHTMLDocument();
> match = doc.querySelectorAll('#coordinates')
[ null, item: [Function: item] ]
> match.length
1
> match[0]
null
> match = doc.querySelectorAll('#coordinates, span')
[ item: [Function: item] ]
> match.length
0

Yeah, that's a bug.

niedzielski commented 7 years ago

This appears to still be an issue (for any selector) on v1.0.29:

$ node
> console.log(require('domino').createDocument().querySelectorAll('h1'))
{ root: 
   { nodeType: 9,
     isHTML: true,
     _address: 'about:blank',
     readyState: 'loading',
     implementation: {},
     ownerDocument: null,
     doctype: 
      { nodeType: 10,
        ownerDocument: [Object],
        name: 'html',
        publicId: '',
        systemId: '',
        _lastModTime: undefined,
        _childNodes: [],
        parentNode: [Object],
        _index: 0,
        _nid: 2 },
     documentElement: 
      HTMLHtmlElement {
        nodeType: 1,
        ownerDocument: [Object],
        localName: 'html',
        namespaceURI: 'http://www.w3.org/1999/xhtml',
        prefix: null,
        tagName: 'HTML',
        childNodes: [Array],
        _attrsByQName: {},
        _attrsByLName: {},
        _attrKeys: [],
        _index: 1,
        parentNode: [Object],
        _nid: 3 },
     childNodes: [ [Object], [Object], item: [Function: item] ],
     _templateDocCache: null,
     _nid: 1,
     _nextnid: 8,
     _nodes: 
      [ null,
        [Object],
        [Object],
        [Object],
        [Object],
        [Object],
        [Object],
        [Object] ],
     byId: {},
     modclock: 1,
     _lastModTime: 1 },
  filter: [Function],
  lastModTime: 1,
  done: true,
  cache: [] }
undefined
cscott commented 7 years ago
> console.log(require('domino').createDocument().querySelectorAll('h1').length)
0
undefined

That seems correct. You are getting an instance of FilteredElementList back.

niedzielski commented 7 years ago

Ah, sorry. The error I'm seeing specifically:

$ node
> const matches = require('domino').createDocument().querySelectorAll('h1')
> console.log(Array.of(matches).length);
1
undefined
cscott commented 7 years ago

I think you want to be using Array.from, not Array.of.

On Sep 30, 2017 4:25 PM, "Stephen Niedzielski" notifications@github.com wrote:

Ah, sorry. The error I'm seeing specifically:

$ node

const matches = require('domino').createDocument().querySelectorAll('h1') console.log(Array.of(matches).length); 1 undefined

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/fgnass/domino/issues/99#issuecomment-333333775, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJhsBNOjGEJWXW-sBCjMvP_7JF6GLzeks5snqOlgaJpZM4Ovwxq .

niedzielski commented 7 years ago

@cscott, my mistake! Thank you!