Skip to content

Commit

Permalink
Fix query string generation methods to only take JsonObjects. (#10068)
Browse files Browse the repository at this point in the history
And some follow up fixes caught by that.

Part of #9876
  • Loading branch information
cramforce authored Jun 21, 2017
1 parent 7937ca5 commit faf9745
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 82 deletions.
7 changes: 4 additions & 3 deletions ads/google/a4a/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {getCorrelator} from './utils';
import {LIFECYCLE_STAGES} from '../../../extensions/amp-a4a/0.1/amp-a4a';
import {dev} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {serializeQueryString} from '../../../src/url';
import {getTimingDataSync} from '../../../src/service/variable-source';
import {urlReplacementsForDoc} from '../../../src/services';
Expand Down Expand Up @@ -46,10 +47,10 @@ import {analyticsForDoc} from '../../../src/analytics';
export class BaseLifecycleReporter {
constructor() {
/**
* @type {!Object<string, string>}
* @type {!JsonObject}
* @private
*/
this.extraVariables_ = new Object(null);
this.extraVariables_ = dict();
}

/**
Expand Down Expand Up @@ -106,7 +107,7 @@ export class BaseLifecycleReporter {
* variables that have been set via #setPingParameter.
*/
reset() {
this.extraVariables_ = new Object(null);
this.extraVariables_ = dict();
}

/**
Expand Down
17 changes: 9 additions & 8 deletions ads/netletix.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import {writeScript, loadScript, validateData} from '../3p/3p';
import {startsWith} from '../src/string.js';
import {dev} from '../src/log.js';
import {dict} from '../src/utils/object';
import {assertHttpsUrl, addParamsToUrl} from '../src/url.js';

const NX_URL_HOST = 'https://call.adadapter.netzathleten-media.de';
Expand Down Expand Up @@ -49,14 +50,14 @@ export function netletix(global, data) {
const url = assertHttpsUrl(
addParamsToUrl(
NX_URL_FULL + encodeURIComponent(data.nxkey || DEFAULT_NX_KEY),
{
unit: data.nxunit || DEFAULT_NX_UNIT,
width: data.nxwidth || DEFAULT_NX_WIDTH,
height: data.nxheight || DEFAULT_NX_HEIGHT,
v: data.nxv || DEFAULT_NX_V,
site: data.nxsite || DEFAULT_NX_SITE,
ord: Math.round(Math.random() * 100000000),
}),
dict({
'unit': data.nxunit || DEFAULT_NX_UNIT,
'width': data.nxwidth || DEFAULT_NX_WIDTH,
'height': data.nxheight || DEFAULT_NX_HEIGHT,
'v': data.nxv || DEFAULT_NX_V,
'site': data.nxsite || DEFAULT_NX_SITE,
'ord': Math.round(Math.random() * 100000000),
})),
data.ampSlotIndex);

window.addEventListener('message', event => {
Expand Down
24 changes: 12 additions & 12 deletions extensions/amp-ad-network-cloudflare-impl/0.1/vendors.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,25 @@
* @const {!JsonObject}
*/
export const NETWORKS = /** @type {!JsonObject} */ ({
cloudflare: {
base: 'https://firebolt.cloudflaredemo.com',
'cloudflare': {
'base': 'https://firebolt.cloudflaredemo.com',
},

adzerk: {
base: 'https://engine.betazerk.com',
'adzerk': {
'base': 'https://engine.betazerk.com',
},

celtra: {
base: 'https://ads-amp.celtra.com',
'celtra': {
'base': 'https://ads-amp.celtra.com',
},

dianomi: {
base: 'https://www.dianomi.com',
src: 'https://www.dianomi.com/smartads.pl?format=a4a',
'dianomi': {
'base': 'https://www.dianomi.com',
'src': 'https://www.dianomi.com/smartads.pl?format=a4a',
},

yieldmo: {
base: 'https://yieldmo-amp.club',
src: 'https://yieldmo-amp.club/ads',
'yieldmo': {
'base': 'https://yieldmo-amp.club',
'src': 'https://yieldmo-amp.club/ads',
},
});
100 changes: 51 additions & 49 deletions extensions/amp-social-share/0.1/amp-social-share-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

import {dict} from '../../../src/utils/object';

/**
* Get social share configurations by supported type.
* @param {string} type
Expand All @@ -24,73 +26,73 @@ export function getSocialConfig(type) {
}

/**
* @type {!Object<string, !Object>}
* @type {!JsonObject}
*/
const BUILTINS = {
twitter: {
shareEndpoint: 'https://twitter.com/intent/tweet',
defaultParams: {
text: 'TITLE',
url: 'CANONICAL_URL',
const BUILTINS = dict({
'twitter': {
'shareEndpoint': 'https://twitter.com/intent/tweet',
'defaultParams': {
'text': 'TITLE',
'url': 'CANONICAL_URL',
},
},
facebook: {
shareEndpoint: 'https://www.facebook.com/dialog/share',
defaultParams: {
href: 'CANONICAL_URL',
'facebook': {
'shareEndpoint': 'https://www.facebook.com/dialog/share',
'defaultParams': {
'href': 'CANONICAL_URL',
},
},
pinterest: {
shareEndpoint: 'https://www.pinterest.com/pin/create/button/',
defaultParams: {
url: 'CANONICAL_URL',
description: 'TITLE',
'pinterest': {
'shareEndpoint': 'https://www.pinterest.com/pin/create/button/',
'defaultParams': {
'url': 'CANONICAL_URL',
'description': 'TITLE',
},
},
linkedin: {
shareEndpoint: 'https://www.linkedin.com/shareArticle',
defaultParams: {
url: 'CANONICAL_URL',
mini: 'true',
'linkedin': {
'shareEndpoint': 'https://www.linkedin.com/shareArticle',
'defaultParams': {
'url': 'CANONICAL_URL',
'mini': 'true',
},
},
gplus: {
shareEndpoint: 'https://plus.google.com/share',
defaultParams: {
url: 'CANONICAL_URL',
'gplus': {
'shareEndpoint': 'https://plus.google.com/share',
'defaultParams': {
'url': 'CANONICAL_URL',
},
},
email: {
shareEndpoint: 'mailto:',
defaultParams: {
subject: 'TITLE',
body: 'CANONICAL_URL',
'email': {
'shareEndpoint': 'mailto:',
'defaultParams': {
'subject': 'TITLE',
'body': 'CANONICAL_URL',
},
},
tumblr: {
shareEndpoint: 'https://www.tumblr.com/share/link',
defaultParams: {
name: 'TITLE',
url: 'CANONICAL_URL',
'tumblr': {
'shareEndpoint': 'https://www.tumblr.com/share/link',
'defaultParams': {
'name': 'TITLE',
'url': 'CANONICAL_URL',
},
},
whatsapp: {
shareEndpoint: 'whatsapp://send',
defaultParams: {
text: 'TITLE - CANONICAL_URL',
'whatsapp': {
'shareEndpoint': 'whatsapp://send',
'defaultParams': {
'text': 'TITLE - CANONICAL_URL',
},
},
sms: {
shareEndpoint: 'sms:',
defaultParams: {
body: 'TITLE - CANONICAL_URL',
'sms': {
'shareEndpoint': 'sms:',
'defaultParams': {
'body': 'TITLE - CANONICAL_URL',
},
},
system: {
shareEndpoint: 'navigator-share:',
defaultParams: {
text: 'TITLE',
url: 'CANONICAL_URL',
'system': {
'shareEndpoint': 'navigator-share:',
'defaultParams': {
'text': 'TITLE',
'url': 'CANONICAL_URL',
},
},
};
});
11 changes: 6 additions & 5 deletions extensions/amp-social-share/0.1/amp-social-share.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {getDataParamsFromAttributes} from '../../../src/dom';
import {getSocialConfig} from './amp-social-share-config';
import {isLayoutSizeDefined} from '../../../src/layout';
import {dev, user} from '../../../src/log';
import {dict} from '../../../src/utils/object';
import {openWindowDialog} from '../../../src/dom';
import {urlReplacementsForDoc} from '../../../src/services';
import {CSS} from '../../../build/amp-social-share-0.1.css';
Expand All @@ -35,8 +36,8 @@ class AmpSocialShare extends AMP.BaseElement {
/** @private {?string} */
this.shareEndpoint_ = null;

/** @private {!Object} */
this.params_ = {};
/** @private @const {!JsonObject} */
this.params_ = dict();

/** @private {?../../../src/service/platform-impl.Platform} */
this.platform_ = null;
Expand Down Expand Up @@ -76,12 +77,12 @@ class AmpSocialShare extends AMP.BaseElement {
return;
}
}
const typeConfig = getSocialConfig(typeAttr) || {};
const typeConfig = getSocialConfig(typeAttr) || dict();
this.shareEndpoint_ = user().assert(
this.element.getAttribute('data-share-endpoint') ||
typeConfig.shareEndpoint,
typeConfig['shareEndpoint'],
'The data-share-endpoint attribute is required. %s', this.element);
this.params_ = Object.assign({}, typeConfig.defaultParams,
Object.assign(this.params_, typeConfig['defaultParams'],
getDataParamsFromAttributes(this.element));
this.platform_ = platformFor(this.win);

Expand Down
5 changes: 3 additions & 2 deletions src/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import {dev} from './log';
import {dict} from './utils/object';
import {cssEscape} from '../third_party/css-escape/css-escape';
import {startsWith} from './string';

Expand Down Expand Up @@ -511,13 +512,13 @@ export function scopedQuerySelectorAll(root, selector) {
* @param {function(string):string=} opt_computeParamNameFunc to compute the parameter
* name, get passed the camel-case parameter name.
* @param {!RegExp=} opt_paramPattern Regex pattern to match data attributes.
* @return {!Object<string, string>}
* @return {!JsonObject}
*/
export function getDataParamsFromAttributes(element, opt_computeParamNameFunc,
opt_paramPattern) {
const computeParamNameFunc = opt_computeParamNameFunc || (key => key);
const dataset = element.dataset;
const params = Object.create(null);
const params = dict();
const paramPattern = opt_paramPattern ? opt_paramPattern : /^param(.+)/;
for (const key in dataset) {
const matches = key.match(paramPattern);
Expand Down
2 changes: 1 addition & 1 deletion src/service/document-info-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ function getPageViewId(win) {
* Returns a map object of link tag relations in document head.
* Key is the link rel, value is a list of corresponding hrefs.
* @param {!Document} doc
* @return {!Object<string, string|!Array<string>>}
* @return {!JsonObject<string, string|!Array<string>>}
*/
function getLinkRels(doc) {
const linkRels = map();
Expand Down
4 changes: 2 additions & 2 deletions src/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export function addParamToUrl(url, key, value, opt_addToFront) {
* Appends query string fields and values to a url. The `params` objects'
* `key`s and `value`s will be transformed into query string keys/values.
* @param {string} url
* @param {!Object<string, string|!Array<string>>} params
* @param {!JsonObject<string, string|!Array<string>>} params
* @return {string}
*/
export function addParamsToUrl(url, params) {
Expand All @@ -186,7 +186,7 @@ export function addParamsToUrl(url, params) {
/**
* Serializes the passed parameter map into a query string with both keys
* and values encoded.
* @param {!Object<string, string|!Array<string>>} params
* @param {!JsonObject<string, string|!Array<string>>} params
* @return {string}
*/
export function serializeQueryString(params) {
Expand Down

0 comments on commit faf9745

Please sign in to comment.