Pin button: hover button broken

[jeherve]

There appears to be an issue with the production code (not the version currently visible in this repo) of the Pin button.

Here are the relevant lines inside showHoverButton:

GitHub repo:

lang: $.f.getData(el, "lang") || $.v.config.lang,

-- https://github.com/pinterest/widgets/blob/b30450562975baecda361d2b4c516441afe53c3e/pinit_main.js#L988

Production, pretty-printed:

lang: e.f.getLang(e.f.getData(d, "lang") || e.v.config.lang || e.v.lang),

It seems that in the GitHub repo (presumably the older code), the getLang call was being done with the first argument to the showHoverButton method, which was an element, and thus worked. In production, the getLang call is being done with some sort of config object instead (d in the minified code; aka the fourth argument that's passed in the IIFE), which fails.

Long explanation Looking at the pretty-printed minified code, the bug happens in a get method:

get: function(a, b) {
  var c = "";
  return c = "string" === typeof a[b] ? a[b] : a.getAttribute(b)
},

This method allows for the first argument to either be an element or an object, so it first checks if the property is available directly (a[b]), and if not it tries to get it with a .getAttribute. However, that's going to fail on objects, since objects don't have getAttribute.

So the idea is that when get is called on an object, the object must have that attribute, or it will throw an error. Okay, that's fair enough.

So what's actually failing? Well, this is failing on a call to getData(d, "lang"), where d is an object. So let's look at an excerpt of d, the object in question:

{
  [...]
  "dataAttributePrefix": "data-pin-",
  "lang": "en",
  [...]
}

From this we assume that the way things are supposed to work is that for objects, {propertyName} is passed to the get call, whereas for elements it's data-pin-{propertyName} instead, so that .getAttribute works and gets a data attribute. So let's look at getData:

getData: function(a, b) {
  return b = e.a.dataAttributePrefix + b,
  e.f.get(a, b)
},

Everything is controlled by dataAttributePrefix, but it's done unconditionally. For this to work, it would have to be the empty string (so that the resulting property name is the same), but in this case it's set to 'data-pin-'. This results in a missing property in the object, which results in a call to an undefined .getAttribute function.

So where does this d object come from? Should its properties be set with data-pin-{propertyName} instead? Well, as far as I can tell, it's the fourth parameter in the pinit_main.js IIFE, and thus provided by Pinterest themselves:

}(window, document, navigator, {
    [...]
    dataAttributePrefix: "data-pin-",
    "lang": "en",
    [...]
});

This leads me to think that Pinterest have pushed a broken build to production.

Do you think you could take a look at this?

Thank you!

---

Also reported here: https://github.com/Automattic/jetpack/issues/17948

[kentbrew]

We have a fix on its way. Great catch!

[kentbrew]

Fix is out; updating source here shortly.

[jeherve]

Thank you!