wnr / element-resize-detector

Optimized cross-browser resize listener for elements.
MIT License
1.32k stars 118 forks source link

[Unable to reproduce] getComputedStyle().width not string in Embeeded Facebook WebView on Android #74

Closed shtse8 closed 7 years ago

shtse8 commented 7 years ago

I have got a error report from this detector.

Cannot read property 'indexOf' of undefined

Stack Trace

ownerDocument.body || a.ownerDocument.body.contains(a)
}
return !b(a)
}

function k(a) {
  var b = o(a).container.childNodes[0];
  return -1 === getComputedStyle(b).width.indexOf("px")
}

function l() {
    var a = getComputedStyle(b),
      c = {};
    return c.position = a.position, c.width = b.offsetWidth, c.height = b.offsetHeight, c.top = a.top, c.ri

User Information:

Timestamp
24/03/2017 05:15:26
Time On Page
1.832 seconds
More from this Visitor
Visitor Time
24/03/2017 05:15:35
Browser (Raw)
Facebook 93.0.0
Operating System
Android 5.1
Source IP
111.243.58.165
Viewport
360 x 519
shtse8 commented 7 years ago

I am using polyfill.io, which they provide the getComputedStyle polyfill by default if the browser don't support it. it seems they return some differences and cause this error.

shtse8 commented 7 years ago

Reference: https://cdn.polyfill.io/v2/polyfill.js?features=IntersectionObserver,requestAnimationFrame&unknown=polyfill&ua=123

// getComputedStyle
(function (global) {
    function getComputedStylePixel(element, property, fontSize) {
        var
        // Internet Explorer sometimes struggles to read currentStyle until the element's document is accessed.
        value = element.document && element.currentStyle[property].match(/([\d\.]+)(%|cm|em|in|mm|pc|pt|)/) || [0, 0, ''],
        size = value[1],
        suffix = value[2],
        rootSize;

        fontSize = !fontSize ? fontSize : /%|em/.test(suffix) && element.parentElement ? getComputedStylePixel(element.parentElement, 'fontSize', null) : 16;
        rootSize = property == 'fontSize' ? fontSize : /width/i.test(property) ? element.clientWidth : element.clientHeight;

        return suffix == '%' ? size / 100 * rootSize :
               suffix == 'cm' ? size * 0.3937 * 96 :
               suffix == 'em' ? size * fontSize :
               suffix == 'in' ? size * 96 :
               suffix == 'mm' ? size * 0.3937 * 96 / 10 :
               suffix == 'pc' ? size * 12 * 96 / 72 :
               suffix == 'pt' ? size * 96 / 72 :
               size;
    }

    function setShortStyleProperty(style, property) {
        var
        borderSuffix = property == 'border' ? 'Width' : '',
        t = property + 'Top' + borderSuffix,
        r = property + 'Right' + borderSuffix,
        b = property + 'Bottom' + borderSuffix,
        l = property + 'Left' + borderSuffix;

        style[property] = (style[t] == style[r] && style[t] == style[b] && style[t] == style[l] ? [ style[t] ] :
                           style[t] == style[b] && style[l] == style[r] ? [ style[t], style[r] ] :
                           style[l] == style[r] ? [ style[t], style[r], style[b] ] :
                           [ style[t], style[r], style[b], style[l] ]).join(' ');
    }

    // <CSSStyleDeclaration>
    function CSSStyleDeclaration(element) {
        var
        style = this,
        currentStyle = element.currentStyle,
        fontSize = getComputedStylePixel(element, 'fontSize'),
        unCamelCase = function (match) {
            return '-' + match.toLowerCase();
        },
        property;

        for (property in currentStyle) {
            Array.prototype.push.call(style, property == 'styleFloat' ? 'float' : property.replace(/[A-Z]/, unCamelCase));

            if (property == 'width') {
                style[property] = element.offsetWidth + 'px';
            } else if (property == 'height') {
                style[property] = element.offsetHeight + 'px';
            } else if (property == 'styleFloat') {
                style.float = currentStyle[property];
            } else if (/margin.|padding.|border.+W/.test(property) && style[property] != 'auto') {
                style[property] = Math.round(getComputedStylePixel(element, property, fontSize)) + 'px';
            } else if (/^outline/.test(property)) {
                // errors on checking outline
                try {
                    style[property] = currentStyle[property];
                } catch (error) {
                    style.outlineColor = currentStyle.color;
                    style.outlineStyle = style.outlineStyle || 'none';
                    style.outlineWidth = style.outlineWidth || '0px';
                    style.outline = [style.outlineColor, style.outlineWidth, style.outlineStyle].join(' ');
                }
            } else {
                style[property] = currentStyle[property];
            }
        }

        setShortStyleProperty(style, 'margin');
        setShortStyleProperty(style, 'padding');
        setShortStyleProperty(style, 'border');

        style.fontSize = Math.round(fontSize) + 'px';
    }

    CSSStyleDeclaration.prototype = {
        constructor: CSSStyleDeclaration,
        // <CSSStyleDeclaration>.getPropertyPriority
        getPropertyPriority: function () {
            throw new Error('NotSupportedError: DOM Exception 9');
        },
        // <CSSStyleDeclaration>.getPropertyValue
        getPropertyValue: function (property) {
            return this[property.replace(/-\w/g, function (match) {
                return match[1].toUpperCase();
            })];
        },
        // <CSSStyleDeclaration>.item
        item: function (index) {
            return this[index];
        },
        // <CSSStyleDeclaration>.removeProperty
        removeProperty: function () {
            throw new Error('NoModificationAllowedError: DOM Exception 7');
        },
        // <CSSStyleDeclaration>.setProperty
        setProperty: function () {
            throw new Error('NoModificationAllowedError: DOM Exception 7');
        },
        // <CSSStyleDeclaration>.getPropertyCSSValue
        getPropertyCSSValue: function () {
            throw new Error('NotSupportedError: DOM Exception 9');
        }
    };

    // <Global>.getComputedStyle
    global.getComputedStyle = function getComputedStyle(element) {
        return new CSSStyleDeclaration(element);
    };
}(this));
shtse8 commented 7 years ago

I have excluded that getComputedStyle polyfill, but the error is still reported by users.

undefined is not an object (evaluating 'getComputedStyle(b).width.indexOf')
Has Stacktrace User: iOS Facebook User@190.197.80.114 Facebook iOS 190.197.80.114

Do you think adding a checking in the following code? e.g. if getComputedStyle(b).width is undefined

function k(a) {
  var b = o(a).container.childNodes[0];
  return -1 === getComputedStyle(b).width.indexOf("px")
}
wnr commented 7 years ago

Hi and thanks for the report. Hmm that is interesting. I don't think the algorithm would work very well if the width is undefined. I would like to reproduce this myself and experiment with it, do you know how to reproduce it?

shtse8 commented 7 years ago

Sorry, I cannot reproduce the iOS problem (I don't have iOS + Facebook device) since it is the error reported from trackjs.com.

Maybe we should avoid the error first and do nothing if the width is undefined. If you just want to reproduce the problem with polyfill, put the following polyfill before other scripts. https://cdn.polyfill.io/v2/polyfill.js?features=IntersectionObserver,requestAnimationFrame&unknown=polyfill&ua=123 I find that they polyfill the getComputedStyle. Then, using latest chrome. I have tested and can see the error. Also, I have reported an issue to their github. https://github.com/Financial-Times/polyfill-service/issues/1138

wnr commented 7 years ago

Okay great, thanks. I'll experiment with the polyfill and fill in some details in the issue if I find anything. I'm not a big fan of having element-resize-detector doing nothing if for some reason the elements in a browser doesn't report a width. It seems to me that the element resize detector should not be used in the first case for such browsers. I'll report back my findings with the polyfill.

wnr commented 7 years ago

It seems indeed like a problem with the polyfill, I'm getting this error simply including it in my html file:

"Error: Failed to execute 'appendChild' on 'Node': Only one element on document allowed.
    at https://cdn.polyfill.io/v2/polyfill.js?features=IntersectionObserver,requestAnimationFrame&unknown=polyfill&ua=123:385:23
    at https://cdn.polyfill.io/v2/polyfill.js?features=IntersectionObserver,requestAnimationFrame&unknown=polyfill&ua=123:473:2
    at https://cdn.polyfill.io/v2/polyfill.js?features=IntersectionObserver,requestAnimationFrame&unknown=polyfill&ua=123:1579:2"

Then, when calling getComputedStyle(document.body) I only get this very limited result CSSStyleDeclaration {margin: "", padding: "", border: "", fontSize: "0px"} which seems like a HTML DOM spec violation to me.

shtse8 commented 7 years ago

I have tried not to use polyfill, but still received some error reports from some users using Facebook iOS.

wnr commented 7 years ago

I have now tried to run the examples in Facebook's WebView on my iOS 10.2, and I'm unable to reproduce it. Have you more information about how I can reproduce this?

shtse8 commented 7 years ago

I am checking the error reports from my sites. If I have got any further information, I will post it here.

wnr commented 7 years ago

Okay, great!

wnr commented 7 years ago

I have pushed an experimental fix to the develop branch, which might solve the issue. Could you try it out?

wnr commented 7 years ago

Did you get any chance to try this out? It seems to work fine for me with the fix in the develop branch.

shtse8 commented 7 years ago

Sorry for my late reply, I have tried the fix and it seems to work fine.

wnr commented 7 years ago

No problems at all :) Great, thanks! This will be released as 1.1.12 soon.

wnr commented 7 years ago

Fix released in 1.1.12. Thanks for the collaboration.