Skip to content

Commit

Permalink
Fixed a bug with my recent PR: ampproject#5621.
Browse files Browse the repository at this point in the history
hasOwn doesn't exist when the object is created using `map()`. Added a
test as well.
  • Loading branch information
avimehta committed Dec 8, 2016
1 parent 36bc1df commit c241a43
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
4 changes: 3 additions & 1 deletion extensions/amp-analytics/0.1/amp-analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ variableServiceFor(AMP.win);

const MAX_REPLACES = 16; // The maximum number of entries in a extraUrlParamsReplaceMap

const hasOwn = Object.prototype.hasOwnProperty;

export class AmpAnalytics extends AMP.BaseElement {

/** @param {!AmpElement} element */
Expand Down Expand Up @@ -570,7 +572,7 @@ export class AmpAnalytics extends AMP.BaseElement {
user().assert(opt_predefinedConfig || property != 'iframePing',
'iframePing config is only available to vendor config.');
// Only deal with own properties.
if (from.hasOwnProperty(property)) {
if (hasOwn.call(from, property)) {
if (isArray(from[property])) {
if (!isArray(to[property])) {
to[property] = [];
Expand Down
3 changes: 3 additions & 0 deletions extensions/amp-analytics/0.1/test/test-amp-analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {markElementScheduledForTesting} from '../../../../src/custom-element';
import {installCidService,} from
'../../../../extensions/amp-analytics/0.1/cid-impl';
import {urlReplacementsForDoc} from '../../../../src/url-replacements';
import {map} from '../../../../src/types';
import * as sinon from 'sinon';

import {AmpDocSingle} from '../../../../src/service/ampdoc-impl';
Expand Down Expand Up @@ -386,6 +387,8 @@ describe('amp-analytics', function() {

return analytics.layoutCallback().then(() => {
expect(analytics.mergeObjects_({}, {})).to.deep.equal({});
expect(analytics.mergeObjects_(map({'a': 0}), map({'b': 1})))
.to.deep.equal(map({'a': 0, 'b': 1}));
expect(analytics.mergeObjects_({'foo': 1}, {'1': 1}))
.to.deep.equal({'foo': 1, '1': 1});
expect(analytics.mergeObjects_({'1': 1}, {'bar': 'bar'}))
Expand Down

0 comments on commit c241a43

Please sign in to comment.