Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

on-click doesn't work with bootstrap and jQuery #625

Closed
kielni opened this issue Jul 10, 2014 · 19 comments
Closed

on-click doesn't work with bootstrap and jQuery #625

kielni opened this issue Jul 10, 2014 · 19 comments
Labels

Comments

@kielni
Copy link

kielni commented Jul 10, 2014

jQuery v2.0.3
Bootstrap v3.0.3

The on-click seems to be sending a wrapped object that causes problems when jQuery tries to look for other click handlers because it doesn't have a getAttribute method.

I have a custom element like this, on a page that includes Bootstrap and jQuery.

<link rel="import" href="../bower_components/polymer/polymer.html">
<polymer-element name="chart-placeholder">
<template>
    <div class="fa fa-bar-chart-o" style="font-size:200px; cursor:pointer" 
        on-click="{{clickHandler}}"></div>
</template>
<script>
Polymer('chart-placeholder', {
    clickHandler: function(event, detail, sender) {
        this.fire('core-signal', {name: "chartClicked", data: "xy"});
    }
});
</script>
</polymer-element>

When I click it in Firefox 29, I get an error every time in Sizzle.attr = function( elem, name ): elem.getAttribute is not a function

elem is Document { impl=document, treeScope_={...}, documentElement=GeneratedWrapper, more...}
There is no elem.getAttribute method on Document. I thought the event handlers weren't supposed to see the wrapped elements?

Here's the stack trace:

Sizzle.attr(elem=Document { impl=document, treeScope_={...}, documentElement=GeneratedWrapper, more...}) jquery.js (line 1707)
Sizzle.selectors.filter.ATTR/<(elem=Document { impl=document, treeScope_={...}, documentElement=GeneratedWrapper, more...}, context=Document 1261204759648, xml=false) jquery.js (line 1900)
elementMatcher/<(seed=[Document { impl=document, treeScope_={...}, documentElement=GeneratedWrapper, more...}], context=Document 1261204759648, xml=false, results=[], expandContext=false) jquery.js (line 2395)
matcherFromGroupMatchers/superMatcher() jquery.js (line 2602)
select() jquery.js (line 2769)
Sizzle() jquery.js (line 1154)
jQuery.event.handlers() jquery.js (line 4721)
jQuery.event.dispatch() jquery.js (line 4658)
jQuery.event.add/elemData.handle() jquery.js (line 4360)
invoke() events.js (line 404)
dispatchBubbling() events.js (line 323)
dispatchEvent() events.js (line 287)
dispatchOriginalEvent()

I can reproduce it by adding Bootstrap and jQuery to the tutorial's index.html (after polymer.js); the on-click in post-card.html does the same thing.

@arv
Copy link
Contributor

arv commented Jul 10, 2014

So Document does not have a getAttribute. That is the correct behavior. The question is whether we for some reason are returning a Document when we should return an Element or if this is an error in jQuery/Sizzle?

@kielni
Copy link
Author

kielni commented Jul 10, 2014

This is the Sizzle.attr method that's failing:

Sizzle.attr = function( elem, name ) {
    // Set document vars if needed
    if ( ( elem.ownerDocument || elem ) !== document ) {
        setDocument( elem );
    }

    var fn = Expr.attrHandle[ name.toLowerCase() ],
        // Don't get fooled by Object.prototype properties (jQuery #13807)
        val = fn && hasOwn.call( Expr.attrHandle, name.toLowerCase() ) ?
            fn( elem, name, !documentIsHTML ) :
            undefined;

    return val === undefined ?
        support.attributes || !documentIsHTML ?
            elem.getAttribute && elem.getAttribute( name ) :
            (val = elem.getAttributeNode(name)) && val.specified ?
                val.value :
                null :
        val;

It looks like it's getting the wrong type of element. The elem.nodeName when it crashes is #document -- that's a shadow DOM node, right? Why is getting that? I don't see this error on a page without polymer.js. It could be something strange about how bootstrap.js adds event handlers?

I can hack around it by adding elem.getAttribute && elem.getAttribute( name ) but I'm kinda uncomfortable messing with jQuery internals for something like this.

@arv
Copy link
Contributor

arv commented Jul 10, 2014

The elem.nodeName when it crashes is #document -- that's a shadow DOM node, right?

No, that is just a Document (like window.document). The parentNode of the <html> element is a Document.

The suspicious thing is that it gets here. Sizzle should never walk past the documentElement (the root element).

@tjsavage tjsavage added the p1 label Aug 21, 2014
@tjsavage tjsavage added this to the beta milestone Aug 21, 2014
@nazar-pc
Copy link
Contributor

I have errors with getAttribute() as well when using jQuery and UIkit.

@nazar-pc
Copy link
Contributor

I found that getEventPath() catches document when bubbling, if cancel that. Is it correct behavior, or not?

@nazar-pc
Copy link
Contributor

nazar-pc commented Sep 1, 2014

I think it is not correct, because I've compared how bubbling works in Firefox without Platform, and it stops on <html>, but when Platform is included - it goes upper, and stops on document.

@nazar-pc
Copy link
Contributor

nazar-pc commented Sep 1, 2014

Just made reproducible case: http://jsfiddle.net/0oekevob/
Move around on result section and see what is in console.
It can be reproduced in Firefox and I think any other browser besides latest versions of Chromium where there is no native support for Web Components and imports.

@nazar-pc
Copy link
Contributor

nazar-pc commented Sep 1, 2014

I found fix for this, nevertheless, it should be fixed in Polymer itself.
Fix is to replace everywhere subscribing to events like:

$(document).on('event', 'element', func)

to

$('html').on('event', 'element', func)

and obviously do not forget to rewrite .off() as well.
This doesn't break anything, since only <html> is inside document, and all elements you may want to catch events on are inside <html>

@smasala
Copy link

smasala commented Oct 16, 2014

what's the status here?

@smasala
Copy link

smasala commented Oct 16, 2014

I don't have the problem with jQuery 2.1.1... with me its when I include bootstrap 3.2.0

@nazar-pc
Copy link
Contributor

I'm using fix I mentioned before, also my pull request with similar fix was accepted in UIkit which I'm using instead of bootstrap, so, probably, it was fixed in bootstrap too, can't confirm that.
@kielni, can you confirm that it was fixed in bootstrap?

@smasala
Copy link

smasala commented Oct 16, 2014

I have the latest bootstrap and it doesn't work. I placed the bootstrap JS code inside the following function which fixes it:

(function(window, document){
    //bootstrap.js - bootstrap.min.js
})(wrap(window), wrap(document));

The only thing is if I update bootstrap with bower, then I have to maintain it - which seems silly.

Alternatively I loaded the jQuery and bootstrap before platform.js - This gives me a warning but not an error.

@tuespetre
Copy link

Here is a workaround that does not require modifying bootstrap's source:

<script src="/path/to/webcomponents.js"></script>
<script src="/path/to/polymer.js"></script>
<script src="/path/to/jquery-2.1.1.min.js"></script>
<script>
    jQuery.ajax({
        url: '/path/to/bootstrap.js',
        success: function (data) {
            (new Function('document', 'window', data))
                .call({}, wrap(document), wrap(window));
        },
        dataType: 'text'
    });
</script>

@drarmstr
Copy link

drarmstr commented Dec 2, 2014

Is this issue related or the same issue as Bootstrap dropdowns and other interactive elements not working from within polymer components?

@tuespetre 's workaround above either globally or within the component doesn't seem to affect the dropdowns from not working.

@martingust
Copy link

Slight modification of @smasala's fix did it for me with webcomponents.js (v0.5.1).

(function(document){
    //bootstrap.js - bootstrap.min.js
})(wrap(document));

@Nevraeka
Copy link

@martingust & @arv - Can we close this issue or do more code fixes need to be applied?

@martingust
Copy link

@Nevraeka that is all I needed to do to get it to work.

@robdodson
Copy link
Contributor

Closing. I've been told that there's no way for Polymer to wrap document on your behalf, it must be done by the developer at runtime.

@HaykoKoryun
Copy link
Contributor

A tedious workaround I just used to get around this annoying problem was to call evt.stopPropagation and return false;. It might not work for all cases but for buttons and things that you know won't need to bubble their events upwards it's a good enough solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.