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.

Sign up for GitHub

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

🐛 Force bust cached document for amp-addthis #26761

Merged
merged 8 commits into from
Feb 12, 2020
Merged

🐛 Force bust cached document for amp-addthis #26761

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
526efe3
Force bust cached document for amp-addthis
alanorozco Feb 12, 2020
bb917ca
escape
alanorozco Feb 12, 2020
ceae5f5
actually escape
alanorozco Feb 12, 2020
0ca931f
URL value is constant string on release, no need for utils
alanorozco Feb 12, 2020
01206c0
unused
alanorozco Feb 12, 2020
9b060a1
Escape ORIGIN
alanorozco Feb 12, 2020
c53fa3b
rewrap for sanity
alanorozco Feb 12, 2020
6669552
LGTM analysis, you're killing me
alanorozco Feb 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions extensions/amp-addthis/0.1/amp-addthis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import {DwellMonitor} from './addthis-utils/monitors/dwell-monitor';
import {PostMessageDispatcher} from './post-message-dispatcher';
import {ScrollMonitor} from './addthis-utils/monitors/scroll-monitor';
import {Services} from '../../../src/services';

import {callEng} from './addthis-utils/eng';
import {callLojson} from './addthis-utils/lojson';
import {callPjson} from './addthis-utils/pjson';
Expand All @@ -66,6 +65,7 @@ import {
} from './addthis-utils/mode';
import {getOgImage} from './addthis-utils/meta';
import {getWidgetOverload} from './addthis-utils/get-widget-id-overloaded-with-json-for-anonymous-mode';
import {internalRuntimeVersion} from '../../../src/internal-version';
import {isLayoutSizeDefined} from '../../../src/layout';
import {listen} from '../../../src/event-helper';
import {parseUrlDeprecated} from '../../../src/url';
Expand Down Expand Up @@ -299,7 +299,10 @@ class AmpAddThis extends AMP.BaseElement {
dict({
'frameborder': 0,
'title': ALT_TEXT,
'src': `${ORIGIN}/dc/amp-addthis.html`,
// Document has overly long cache age: go.amp.dev/issue/24848
// Adding AMP runtime version as a meaningless query param to force bust
// cached versions.
'src': `${ORIGIN}/dc/amp-addthis.html?_amp_=${internalRuntimeVersion()}`,
'id': this.widgetId_,
'pco': this.productCode_,
'containerClassName': this.containerClassName_,
Expand Down
9 changes: 6 additions & 3 deletions extensions/amp-addthis/0.1/test/test-amp-addthis.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {getDetailsForMeta, getMetaElements} from './../addthis-utils/meta';
import {getKeywordsString} from './../addthis-utils/classify';
import {getSessionId} from '../addthis-utils/session';
import {getWidgetOverload} from '../addthis-utils/get-widget-id-overloaded-with-json-for-anonymous-mode';
import {startsWith} from '../../../../src/string';
import {toArray} from '../../../../src/types';

describes.realWin(
Expand Down Expand Up @@ -101,9 +102,11 @@ describes.realWin(

function testIframe(iframe) {
expect(iframe).to.not.equal(null);
expect(iframe.getAttribute('src')).to.equal(
`${ORIGIN}/dc/amp-addthis.html`
);
const srcPrefix = `${ORIGIN}/dc/amp-addthis.html?`;
expect(
startsWith(iframe.getAttribute('src'), srcPrefix),
`iframe src starts with ${srcPrefix}`
).to.be.true;
expect(iframe.getAttribute('title')).to.equal(ALT_TEXT);
}

Expand Down