metafizzy / outlayer

:construction_worker: the brains & guts of a layout library
163 stars 63 forks source link

HTMLElement is always a function #16

Closed Waterlog closed 10 years ago

Waterlog commented 10 years ago

https://github.com/desandro/masonry/issues/524

Waterlog commented 10 years ago

Original function:

var isElement = ( typeof HTMLElement === 'object' ) ?
  function isElementDOM2( obj ) {
    return obj instanceof HTMLElement;
  } :
  function isElementQuirky( obj ) {
    return obj && typeof obj === 'object' &&
      obj.nodeType === 1 && typeof obj.nodeName === 'string';
  };

1) typeof HTMLElement === 'object' returns false in Chrome, while typeof HTMLElement === 'function' returns true.

If I use typeof HTMLElement === 'object', isElementQuirky function is always used, which works well for Chrome both on a page load and for an appended content.

When I use typeof HTMLElement === 'function', Chrome stops working when isElementDOM2 returns false. On a page load isElementDOM2 returns true, while for appended content it returns false (obj instanceof HTMLElement).

So it looks like we should always use a second function for Chrome.

2) typeof HTMLElement === 'object' returns true on iOS, while typeof HTMLElement === 'function' returns false.

If I use typeof HTMLElement === 'function', isElementQuirky function is always used, which works well for iOS both on a page load and for an appended content.

When I use typeof HTMLElement === 'object', iOS stops working when isElementDOM2 returns false. On a page load isElementDOM2 returns true, while for appended content it returns false (obj instanceof HTMLElement).

3) So the issue is, isElementDOM2 function returns false on appended content.

4) ( typeof HTMLElement === 'function' || typeof HTMLElement === 'object' ) This code doesn't fix an issue, since it forces us to use isElementDOM2 function, which returns false on appended content on both Chrome and iOS - as a result, we get a completely broken appended function.

5) I suppose a right fix is the isElementDOM2 function removal - this solution works for both iOS and Chrome, not sure about the other browsers. Here's a code:

var isElement = function isElementQuirky( obj ) {
    return obj && typeof obj === 'object' && obj.nodeType === 1 && typeof obj.nodeName === 'string';
};
desandro commented 10 years ago

I'm unable to reproduce isElementDOM2 returning false for both iOS and Chrome. See example: http://codepen.io/desandro/pen/lAshF/

Waterlog commented 10 years ago

Note that I've tested this directly inside Masonry, by using alerts, with Chrome on my desktop and iPhone in my hand. Your example at least doesn't use a document fragment. Current test case shows a result of the first inititation only, which works on both iOS and Chrome.

Btw, try to push a black button at the bottom side of the screen at http://start-germany.ru/ on your iOS to see the issue.

desandro commented 10 years ago

Ah, thanks for sharing. I took a look at the site, and I'm not sure where the problem is. It appears you're using the same kind of code as in the Masonry appended example. This uses document.fragment. http://codepen.io/desandro/pen/nhekz So I'm not sure if these two issues are related.

Waterlog commented 10 years ago

Here's where a problem is https://github.com/metafizzy/outlayer/pull/16#issuecomment-38028550

Waterlog commented 10 years ago

I'll test this code in many other browsers in a couple of days (up to ie8) and will alert you if a solution of the function removal doesn't work somewhere.

Since there's no issue in my code, a current version of Masonry doesn't work on iOS, when height of the items isn't fixed.

desandro commented 10 years ago

I chose to resolve this with the above commit. Thanks again for alerting me to this issue!