From 44a18b05636c85521dcab2c6105336f3676da828 Mon Sep 17 00:00:00 2001
From: Corey Masanto <masanto@google.com>
Date: Thu, 21 Oct 2021 14:19:06 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20[amp-story-page-outlink]=20Force?=
 =?UTF-8?q?=20page=20outlinks=20to=20use=20target=3D'=5Ftop'=20in=20order?=
 =?UTF-8?q?=20to=20prevent=20navigation=20from=20breaking=20on=20Safari=20?=
 =?UTF-8?q?(#36428)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Force page outlinks to use target='_top' in order to prevent them from breaking on Safari

* Lint

* Add test-amp-story-page-attachment.js with basic attachment and outlink tests, along with a test verifying that outlink anchor elements always have a value of '_top'

* Lint
---
 .../1.0/amp-story-page-attachment.js          | 15 +++--
 .../test/test-amp-story-page-attachment.js    | 56 +++++++++++++++++++
 2 files changed, 67 insertions(+), 4 deletions(-)
 create mode 100644 extensions/amp-story/1.0/test/test-amp-story-page-attachment.js

diff --git a/extensions/amp-story/1.0/amp-story-page-attachment.js b/extensions/amp-story/1.0/amp-story-page-attachment.js
index 2d8abd8426f3..9a4527af3eff 100644
--- a/extensions/amp-story/1.0/amp-story-page-attachment.js
+++ b/extensions/amp-story/1.0/amp-story-page-attachment.js
@@ -213,11 +213,18 @@ export class AmpStoryPageAttachment extends DraggableDrawer {
         <svg class="i-amphtml-story-page-attachment-remote-icon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48"><path d="M38 38H10V10h14V6H10c-2.21 0-4 1.79-4 4v28c0 2.21 1.79 4 4 4h28c2.21 0 4-1.79 4-4V24h-4v14zM28 6v4h7.17L15.51 29.66l2.83 2.83L38 12.83V20h4V6H28z"></path></svg>
       </a>`;
 
+    const isPageOutlink = this.element.tagName === 'AMP-STORY-PAGE-OUTLINK';
+    if (isPageOutlink) {
+      // The target must be '_top' for page outlinks, which will result in the
+      // link opening in the current tab. Opening links in a new tab requires a
+      // trusted event, and Safari does not consider swiping up to be trusted.
+      this.element.querySelector('a').setAttribute('target', '_top');
+    }
+
     // For backwards compatibility if element is amp-story-page-outlink.
-    const hrefAttr =
-      this.element.tagName === 'AMP-STORY-PAGE-OUTLINK'
-        ? this.element.querySelector('a').getAttribute('href')
-        : this.element.getAttribute('href');
+    const hrefAttr = isPageOutlink
+      ? this.element.querySelector('a').getAttribute('href')
+      : this.element.getAttribute('href');
 
     // URL will be validated and resolved based on the canonical URL if relative
     // when navigating.
diff --git a/extensions/amp-story/1.0/test/test-amp-story-page-attachment.js b/extensions/amp-story/1.0/test/test-amp-story-page-attachment.js
new file mode 100644
index 000000000000..ee7f402508c2
--- /dev/null
+++ b/extensions/amp-story/1.0/test/test-amp-story-page-attachment.js
@@ -0,0 +1,56 @@
+import {AmpDocSingle} from '#service/ampdoc-impl';
+import {AmpStoryPageAttachment} from '../amp-story-page-attachment';
+
+describes.realWin('amp-story-page-attachment', {amp: true}, (env) => {
+  let attachmentEl;
+  let attachment;
+  let outlinkEl;
+  let outlink;
+
+  beforeEach(() => {
+    const {win} = env;
+
+    // Set up the story.
+    const storyEl = win.document.createElement('amp-story');
+    storyEl.getAmpDoc = () => new AmpDocSingle(win);
+    win.document.body.appendChild(storyEl);
+
+    // Set up the attachment element for inline attachment testing.
+    attachmentEl = win.document.createElement('amp-story-page-attachment');
+    attachmentEl.getAmpDoc = () => new AmpDocSingle(win);
+    storyEl.appendChild(attachmentEl);
+    attachment = new AmpStoryPageAttachment(attachmentEl);
+
+    // Set up the outlink element for outlink testing.
+    outlinkEl = win.document.createElement('amp-story-page-outlink');
+    outlinkEl.getAmpDoc = () => new AmpDocSingle(win);
+    storyEl.appendChild(outlinkEl);
+    outlinkEl.appendChild(win.document.createElement('a'));
+    outlink = new AmpStoryPageAttachment(outlinkEl);
+  });
+
+  afterEach(() => {
+    attachmentEl.remove();
+    outlinkEl.remove();
+  });
+
+  it('should build an attachment', async () => {
+    attachment.buildCallback();
+    return attachment.layoutCallback();
+  });
+
+  it('should build an outlink', async () => {
+    outlink.buildCallback();
+    return outlink.layoutCallback();
+  });
+
+  it('should build amp-story-page-outlink with target="_top" even when the publisher has specified a different value', async () => {
+    const anchorEl = outlinkEl.querySelector('amp-story-page-outlink a');
+    anchorEl.setAttribute('target', '_blank');
+
+    outlink.buildCallback();
+    await outlink.layoutCallback();
+
+    expect(anchorEl.getAttribute('target')).to.eql('_top');
+  });
+});