Fix wrong context.location in ampcontext (#10631)

* Fix wrong context.location in ampcontext * fix test
ampproject · Jul 25, 2017 · 8de49a7 · 8de49a7
1 parent 952b033
commit 8de49a7
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 14 deletions.
diff --git a/3p/ampcontext.js b/3p/ampcontext.js
@@ -21,6 +21,7 @@ import {nextTick} from './3p';
 import {tryParseJson} from '../src/json';
 import {isObject} from '../src/types';
 import {AmpEvents} from '../src/amp-events';
+import {parseUrl} from '../src/url';
 
 export class AbstractAmpContext {
 
@@ -256,7 +257,7 @@ export class AbstractAmpContext {
     this.hidden = context.hidden;
     this.initialLayoutRect = context.initialLayoutRect;
     this.initialIntersection = context.initialIntersection;
-    this.location = context.location;
+    this.location = parseUrl(context.location.href);
     this.mode = context.mode;
     this.pageViewId = context.pageViewId;
     this.referrer = context.referrer;

diff --git a/test/functional/test-amp-context.js b/test/functional/test-amp-context.js
@@ -65,8 +65,18 @@ describe('3p ampcontext.js', () => {
     win.name = generateSerializedAttributes();
     const context = new AmpContext(win);
     expect(context).to.be.ok;
-    expect(context.location).to.equal('foo.com');
-    expect(context.canonicalUrl).to.equal('foo.com');
+    expect(context.location).to.deep.equal({
+      'hash': '',
+      'host': 'foo.com',
+      'hostname': 'foo.com',
+      'href': 'https://foo.com/a?b=c',
+      'origin': 'https://foo.com',
+      'pathname': '/a',
+      'port': '',
+      'protocol': 'https:',
+      'search': '?b=c',
+    });
+    expect(context.canonicalUrl).to.equal('https://bar.com');
     expect(context.pageViewId).to.equal('1');
     expect(context.sentinel).to.equal('1-291921');
     expect(context.startTime).to.equal(0);
@@ -77,8 +87,18 @@ describe('3p ampcontext.js', () => {
     win.name = generateSerializedAttributesA4A();
     const context = new AmpContext(win);
     expect(context).to.be.ok;
-    expect(context.location).to.equal('foo.com');
-    expect(context.canonicalUrl).to.equal('foo.com');
+    expect(context.location).to.deep.equal({
+      'hash': '',
+      'host': 'foo.com',
+      'hostname': 'foo.com',
+      'href': 'https://foo.com/a?b=c',
+      'origin': 'https://foo.com',
+      'pathname': '/a',
+      'port': '',
+      'protocol': 'https:',
+      'search': '?b=c',
+    });
+    expect(context.canonicalUrl).to.equal('https://bar.com');
     expect(context.pageViewId).to.equal('1');
     expect(context.sentinel).to.equal('1-291921');
     expect(context.startTime).to.equal(0);
@@ -89,8 +109,18 @@ describe('3p ampcontext.js', () => {
     win.AMP_CONTEXT_DATA = generateAttributes();
     const context = new AmpContext(win);
     expect(context).to.be.ok;
-    expect(context.location).to.equal('foo.com');
-    expect(context.canonicalUrl).to.equal('foo.com');
+    expect(context.location).to.deep.equal({
+      'hash': '',
+      'host': 'foo.com',
+      'hostname': 'foo.com',
+      'href': 'https://foo.com/a?b=c',
+      'origin': 'https://foo.com',
+      'pathname': '/a',
+      'port': '',
+      'protocol': 'https:',
+      'search': '?b=c',
+    });
+    expect(context.canonicalUrl).to.equal('https://bar.com');
     expect(context.pageViewId).to.equal('1');
     expect(context.sentinel).to.equal('1-291921');
     expect(context.startTime).to.equal(0);
@@ -323,8 +353,10 @@ function generateAttributes(opt_sentinel) {
   name.attributes = {};
   const sentinel = opt_sentinel || '1-291921';
   name.attributes._context = {
-    location: 'foo.com',
-    canonicalUrl: 'foo.com',
+    location: {
+      href: 'https://foo.com/a?b=c',
+    },
+    canonicalUrl: 'https://bar.com',
     pageViewId: '1',
     sentinel,
     startTime: 0,
@@ -342,8 +374,10 @@ function generateAttributesA4A(opt_sentinel) {
   const attributes = {};
   const sentinel = opt_sentinel || '1-291921';
   attributes._context = {
-    location: 'foo.com',
-    canonicalUrl: 'foo.com',
+    location: {
+      href: 'https://foo.com/a?b=c',
+    },
+    canonicalUrl: 'https://bar.com',
     pageViewId: '1',
     sentinel,
     startTime: 0,
@@ -358,8 +392,10 @@ function generateIncorrectAttributes() {
   const name = {};
   name.attributes = {};
   name.attributes.wrong = {
-    location: 'foo.com',
-    canonicalUrl: 'foo.com',
+    location: {
+      href: 'https://foo.com/a?b=c',
+    },
+    canonicalUrl: 'https://foo.com',
     pageViewId: '1',
     sentinel: '1-291921',
     startTime: 0,

diff --git a/test/integration/test-amp-ad-3p.js b/test/integration/test-amp-ad-3p.js
@@ -86,7 +86,17 @@ function createIframeWithApis(fixture) {
     expect(context.initialIntersection.rootBounds).to.be.defined;
     expect(context.isMaster).to.be.defined;
     expect(context.computeInMasterFrame).to.be.defined;
-    expect(context.location).to.be.defined;
+    expect(context.location).to.deep.equal({
+      hash: '',
+      host: 'localhost:9876',
+      hostname: 'localhost',
+      href: 'http://localhost:9876/context.html',
+      origin: 'http://localhost:9876',
+      pathname: '/context.html',
+      port: '9876',
+      protocol: 'http:',
+      search: '',
+    });
     expect(context.sourceUrl).to.be.a('string');
   }).then(() => {
     // test iframe will send out render-start to amp-ad