visible data-bind working in all browsers but Firefox 16+ #707

Closed
robconery opened this Issue Nov 11, 2012 · 5 comments

3 participants

@robconery

Hi guys, on Tekpub.com I've bound our payment selectors to conditionally show the Stripe Button, which is in an iframe. It works just fine in IE, Chrome, and Safari but doesn't work properly in Firefox.

This leads me to think that it's one of two things: a bug in Knockout WRT Firefox, or a bug in how Firefox allows visibility changes to elements containing an IFrame.

If you would like to see the problem in action (which is by far the simplest thing) - put this production in your basket making sure to use Firefox - it's easiest if you're not logged in:

http://tekpub.com/productions/knockout

Then head to the checkout. You should be able to successfully toggle between Paypal and Credit Card.

Now, enter your email address (or any > 3 length value). You should see the Stripe Button (or, sometimes not).

Next, toggle between Paypal and Credit card. The Stripe Button should dissapear on you.

This is a maddening error as it tends to be a bit random. Sometimes the PayPal button dissapears, sometimes it's the Stripe Button.

The code for the page is here:
http://tekpub.com/javascripts/invoicer.js

canCheckout and isCreditCard are the toggles in action here.

I wish I had some way to put this in a test so I could show you failing code - and I wish I had more of a lead on why it's happening. I'm going to keep investigating - but for now this is what I have.

@rniemeyer
Knockout.js member

I will keep trying, but I can't repro this one. Tried on Mac and Windows. FF 16.0.2.

@robconery - can you repro this on a test server? When you inspect the view model are the values correct? Maybe add a <pre data-bind="text: ko.toJSON($root, null, 2)"></pre> near that area.

Just wanted to identify if there is an issue with the view model being updated, the binding being triggered, or the actually way the binding works.

The visible binding adds and removes display: none directly to the element's style, so you can also inspect the various elements when you see this issue and verify that they do not look correct.

Another trick is to create a custom binding called something like logger, then put it on the elements that have the visible binding. All bindings on an element are triggered together, so it will also fire whenever the visible binding fires. Something like:

<div data-bind="visible: showMe, logger: 'show me fired'">test</div>
ko.bindingHandlers.logger = {
    update: function(element, valueAccessor, all, data) {
        var message = ko.utils.unwrapObservable(valueAccessor());
        if (console && console.log) {
              console.log(message, ko.toJS(all()));
        }
    }        
};

http://jsfiddle.net/rniemeyer/x8JJt/

@rniemeyer
Knockout.js member

Also, for the first suggestion of adding a debug section in a pre, you can also create a bookmarklet with something like:

javascript:(function(ko) { var context = ko.contextFor(document.body); if (context && !document.getElementById("_ko_debug")) { var div = document.createElement("div"); div.id = "_ko_debug"; div.innerHTML = '<a href="#" data-bind="click: close">x</a><pre data-bind="text: ko.toJSON(data, null, 2)"></pre>'; var close = function() { ko.removeNode(div); }; document.body.appendChild(div); ko.applyBindings({ data: context.$root, close: close }, div); } })(window.ko);

Run it when you see the issue and scroll to the very bottom of your page to see a debug section. This lets you easily do it on an unmodified site. I suspect that you have already looked at it in the console though, so it might not help get any closer on this issue.

@SteveSanderson

I've tried to repro this, and managed to do so only once (and can't get it to happen again).

Using the Firefox DOM inspector, I noticed that even when the Stripe button wasn't visible, it wasn't actually hidden: it actually just had its iframe size set to 1px by 1px and overflow: hidden. So, the problem isn't KO's visible binding - it's that the iframe is being set to this one-pixel size for some reason.

Digging further, I found the following suspicious code in Stripe's index.js:

App.prototype.renderButton = function() {
    this.button = new Button({
        label: this.options.label
    });
    this.$el.on('active.button', this.activate);
    this.append(this.button);
    return this.invokeParent('resizeButton', this.$el.outerWidth() + 1, this.$el.outerHeight() + 1);
};

I can't prove it, but I suspect this is what is setting the iframe to 1px by 1px. It's well known that jQuery's outerWidth and outerHeight functions return zero when an element is inside a hidden parent, so if this function runs while the button is in a hidden parent, it would resize the button to 1px by 1px.

Further, this function is called through a cross-domain postMessage call. Delivery of these messages is asynchronous, and the timing isn't guaranteed. So this might tie in with it being hard to repro. It may be that the button gets resized to 1px by 1px (effectively invisible) only if the renderButton call happens to land before the parent element has been made visible.

In conclusion, I think:

  1. Stripe shouldn't assume that outerWidth or outerHeight will return a useful nonzero value.
  2. As a workaround for Tekpub, you might need to ensure that none of the chain of parents for your Stripe button ever gets set to display:none. In other words, instead of using the KO visible: someValue binding on a parent, consider using something like css: { pseudoHidden: someValue } instead, defining the CSS class pseudoHidden as visibility: hidden; position: absolute; left: -10000px;. Since this doesn't set display:none, it means jQuery's outerWidth/outerHeight should continue to return nonzero values when called on descendants.

If you try this workaround, could you let us know whether the problem goes away or not?

@SteveSanderson

Closing for now as it appears to be a limitation in the Stripe button, but please reopen with further details if you disagree!

@robconery

Thank you guys very very much! I love the detail here and I've forwarded it to the Stripe team so they can look it over. For now I'm letting it ride to see how much it's really a "problem". I've had 2 or 3 reports that people are vexxed by it so I think I'll just leave it for now.

I really appreciate you looking into it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment