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.

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

✨New action for closing or navigating a window. #19962

Merged
merged 3 commits into from
Dec 21, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions spec/amp-actions-and-events.md
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,10 @@ actions that apply to the whole document.
<td><code>navigateTo(url=STRING, target=STRING, opener=BOOLEAN)</code></td>
<td>Navigates current window to given URL, to the optional specified target if given (currenly only supporting <code>_top</code> and <code>_blank </code>). The optional <code>opener</code> parameter can be specified when using a target of <code>_blank</code> to allow the newly opened page to access <a href="https://developer.mozilla.org/en-US/docs/Web/API/Window/opener"><code>window.opener<code></a>. Supports <a href="https://github.com/ampproject/amphtml/blob/master/spec/amp-var-substitutions.md">standard URL substitutions</a>.</td>
</tr>
<tr>
<td><code>closeOrNavigateTo(url=STRING, target=STRING, opener=BOOLEAN)</code></td>
<td>Tries to close the window if allowed, otherwise it navigates similar to <code>navigateTo</code> Action. Useful for use-cases where a "Back" button may need to close the window if it were opened in a new window from previous page or navigate if it wasn't opened.</td>
</tr>
<tr>
<td><code>goBack</code></td>
<td>Navigates back in history.</td>
Expand Down
84 changes: 67 additions & 17 deletions src/service/standard-actions-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class StandardActions {
if (!invocation.satisfiesTrust(ActionTrust.HIGH)) {
return null;
}
const {node, caller, method, args} = invocation;
const {node, method, args} = invocation;
const win = (node.ownerDocument || node).defaultView;
switch (method) {
case 'pushState':
Expand All @@ -122,22 +122,10 @@ export class StandardActions {
});

case 'navigateTo':
// Some components have additional constraints on allowing navigation.
let permission = Promise.resolve();
if (startsWith(caller.tagName, 'AMP-')) {
permission = caller.getImpl().then(impl => {
if (typeof impl.throwIfCannotNavigate == 'function') {
impl.throwIfCannotNavigate();
}
});
}
return permission.then(() => {
Services.navigationForDoc(this.ampdoc).navigateTo(
win, args['url'], `AMP.${method}`,
{target: args['target'], opener: args['opener']});
}, /* onrejected */ e => {
user().error(TAG, e.message);
});
return this.handleNavigateTo(invocation);

case 'closeOrNavigateTo':
return this.handleCloseOrNavigateTo(invocation);

case 'scrollTo':
user().assert(args['id'],
Expand Down Expand Up @@ -166,6 +154,68 @@ export class StandardActions {
throw user().createError('Unknown AMP action ', method);
}

/**
* Handles the `navigateTo` action.
* @param {!./action-impl.ActionInvocation} invocation
* @return {?Promise}
aghassemi marked this conversation as resolved.
Show resolved Hide resolved
*/
handleNavigateTo(invocation) {
const {node, caller, method, args} = invocation;
const win = (node.ownerDocument || node).defaultView;
// Some components have additional constraints on allowing navigation.
let permission = Promise.resolve();
if (startsWith(caller.tagName, 'AMP-')) {
permission = caller.getImpl().then(impl => {
if (typeof impl.throwIfCannotNavigate == 'function') {
impl.throwIfCannotNavigate();
}
});
}
return permission.then(() => {
Services.navigationForDoc(this.ampdoc).navigateTo(
win, args['url'], `AMP.${method}`,
{target: args['target'], opener: args['opener']});
}, /* onrejected */ e => {
user().error(TAG, e.message);
});
}

/**
* Handles the `handleCloseOrNavigateTo` action.
* This action tries to close the requesting window if allowed, otherwise
* navigates the window.
*
* Window can be closed only from top-level documents that have an opener.
* Without an opener or if embedded, it will deny the close method.
* @param {!./action-impl.ActionInvocation} invocation
* @return {?Promise}
aghassemi marked this conversation as resolved.
Show resolved Hide resolved
*/
handleCloseOrNavigateTo(invocation) {
const {node} = invocation;
const win = (node.ownerDocument || node).defaultView;
let result = Promise.resolve();

// Don't allow closing if embedded in iframe or does not have an opener or
// embedded in a multi-doc shadowDOM case.
// Note that browser denies win.close in some of these cases already anyway,
// so not every check here is strictly needed but works as a short-circuit.
const hasParent = win.parent != win;
const canBeClosed = win.opener && this.ampdoc.isSingleDoc() && !hasParent;

let wasClosed = false;
if (canBeClosed) {
// Browser may still deny win.close() call, that would be reflected
// synchronously in win.closed
win.close();
wasClosed = win.closed;
}

if (!wasClosed) {
result = this.handleNavigateTo(invocation);
aghassemi marked this conversation as resolved.
Show resolved Hide resolved
}

return result;
aghassemi marked this conversation as resolved.
Show resolved Hide resolved
}
/**
* Handles the `scrollTo` action where given an element, we smooth scroll to
* it with the given animation duraiton
Expand Down
89 changes: 88 additions & 1 deletion test/functional/test-standard-actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,94 @@ describes.sandboxed('StandardActions', {}, () => {
});
});

describe('closeOrNavigateTo', () => {
let navigateToStub;
let closeOrNavigateToSpy;
let winCloseSpy;

beforeEach(() => {
navigateToStub = sandbox.stub(standardActions, 'handleNavigateTo')
.returns(Promise.resolve());

closeOrNavigateToSpy = sandbox.spy(standardActions,
'handleCloseOrNavigateTo');

win.close = () => {
aghassemi marked this conversation as resolved.
Show resolved Hide resolved
win.closed = true;
};
winCloseSpy = sandbox.spy(win, 'close');

// Fake ActionInvocation.
invocation.method = 'closeOrNavigateTo';
invocation.args = {
url: 'http://bar.com',
};
invocation.caller = {tagName: 'DIV'};
});

it('should be implemented', () => {
aghassemi marked this conversation as resolved.
Show resolved Hide resolved
return standardActions.handleAmpTarget(invocation).then(() => {
expect(closeOrNavigateToSpy).to.be.calledOnce;
expect(closeOrNavigateToSpy).to.be.calledWithExactly(invocation);
});
});

it('should close window if allowed', () => {
win.opener = {};
win.parent = win;
return standardActions.handleAmpTarget(invocation).then(() => {
expect(winCloseSpy).to.be.calledOnce;
expect(navigateToStub).to.be.not.called;
});
});

it('should NOT close if no opener', () => {
win.opener = null;
win.parent = win;
return standardActions.handleAmpTarget(invocation).then(() => {
expect(winCloseSpy).to.be.not.called;
});
});

it('should NOT close if has a parent', () => {
win.opener = {};
win.parent = {};
return standardActions.handleAmpTarget(invocation).then(() => {
expect(winCloseSpy).to.be.not.called;
});
});

it('should NOT close if in multi-doc', () => {
win.opener = {};
win.parent = win;
sandbox.stub(ampdoc, 'isSingleDoc').returns(false);
return standardActions.handleAmpTarget(invocation).then(() => {
expect(winCloseSpy).to.be.not.called;
});
});

it('should navigate if not allowed to close', () => {
win.opener = null;
win.parent = win;
sandbox.stub(ampdoc, 'isSingleDoc').returns(false);
return standardActions.handleAmpTarget(invocation).then(() => {
expect(winCloseSpy).to.be.not.called;
expect(navigateToStub).to.be.called;
});
});

it('should navigate if win.close rejects', () => {
win.opener = {};
win.parent = win;
win.close = () => {
win.closed = false;
};
return standardActions.handleAmpTarget(invocation).then(() => {
expect(navigateToStub).to.be.called;
});
});
});

it('should implement goBack', () => {
installHistoryServiceForDoc(ampdoc);
const history = Services.historyForDoc(ampdoc);
Expand All @@ -438,7 +526,6 @@ describes.sandboxed('StandardActions', {}, () => {
expect(optoutStub).to.be.calledOnce;
});


it('should implement setState()', () => {
const element = createElement();

Expand Down
17 changes: 17 additions & 0 deletions test/manual/amp-close-or-navigate-target.amp.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!doctype html>
<html ⚡>
<head>
<meta charset="utf-8">
<title>AMP #0</title>
<link rel="canonical" href="amps.html" >
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<link href='https://fonts.googleapis.com/css?family=Questrial' rel='stylesheet' type='text/css'>
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
</head>
<body>
<h1>Target page for AMP.closeOrNavigateTo() manual tests</h1>
<a href="#" on="tap:AMP.closeOrNavigateTo(url='https://example.com')">Back</a>
(this may close the window or navigate to example.com depending on how this page was opened)
</body>
</html>
24 changes: 24 additions & 0 deletions test/manual/amp-close-or-navigate.amp.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!doctype html>
<html ⚡>
<head>
<meta charset="utf-8">
<title>AMP #0</title>
<link rel="canonical" href="amps.html" >
<meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
<link href='https://fonts.googleapis.com/css?family=Questrial' rel='stylesheet' type='text/css'>
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<script async src="https://cdn.ampproject.org/v0.js"></script>
<style amp-custom>
a {
display: block;
margin: 10px;
}
</style>
</head>
<body>
<h1>AMP.closeOrNavigateTo(url=)</h1>
<a href="#" on="tap:AMP.closeOrNavigateTo(url='https://example.com')">This can not close the window so it falls back and navigates to example.com</a>
<a href="amp-close-or-navigate-target.amp.html" target="_blank">This will open a new tab and AMP.closeOrNavigateTo() should close the window there</a>
<a href="amp-close-or-navigate-target.amp.html">This will navigate to another AMP page in the same window and AMP.closeOrNavigateTo() should fail there and navigate to example.com as fallback</a>
</body>
</html>