From 1712081f0b478987df0302a3282c239b4f6e377b Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Sat, 20 Apr 2019 09:12:30 -0700
Subject: [PATCH] fix: fix profile clicks from compose dialog (#1160)

* fix: don't allow profile clicks from compose dialog

fixes #1159

* make the links work correctly in the modal

* fix tests
---
 .../_components/compose/ComposeAuthor.html    | 33 ++++++++++++-------
 .../_components/compose/ComposeBox.html       |  5 +--
 .../dialog/components/ComposeDialog.html      |  2 +-
 tests/spec/029-back-button-modal.js           | 21 ++++++++++--
 tests/spec/118-display-name-custom-emoji.js   | 26 ++++++---------
 tests/utils.js                                |  4 +++
 6 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/src/routes/_components/compose/ComposeAuthor.html b/src/routes/_components/compose/ComposeAuthor.html
index ccd519bf..d9b0c277 100644
--- a/src/routes/_components/compose/ComposeAuthor.html
+++ b/src/routes/_components/compose/ComposeAuthor.html
@@ -1,16 +1,20 @@
 <a {href}
    rel="prefetch"
    class="compose-box-avatar {loaded ? 'loaded' : 'not-loaded'}"
-   aria-hidden={!loaded}
-   aria-label="Profile for {accessibleName}">
+   aria-hidden={true}
+   on:click="onClick(event)"
+>
   <Avatar account={verifyCredentials} size="small"/>
 </a>
 <a class="compose-box-display-name {loaded ? 'loaded' : 'not-loaded'}"
    {href}
    aria-hidden={!loaded}
-   rel="prefetch">
+   rel="prefetch"
+   on:click="onClick(event)"
+>
   <AccountDisplayName account={verifyCredentials} />
 </a>
+
 <span class="compose-box-handle {loaded ? 'loaded' : 'not-loaded'}"
       aria-hidden={!loaded} >
   {'@' + verifyCredentials.acct}
@@ -64,8 +68,9 @@
   import Avatar from '../Avatar.html'
   import { store } from '../../_store/store'
   import AccountDisplayName from '../profile/AccountDisplayName.html'
-  import { removeEmoji } from '../../_utils/removeEmoji'
   import { ONE_TRANSPARENT_PIXEL } from '../../_static/media'
+  import { emit } from '../../_utils/eventBus'
+  import { goto } from '../../../../__sapper__/client'
 
   export default {
     components: {
@@ -86,14 +91,20 @@
         }
       },
       id: ({ verifyCredentials }) => (verifyCredentials && verifyCredentials.id),
-      href: ({ id }) => (id ? `/accounts/${id}` : '#'),
-      emojis: ({ verifyCredentials }) => (verifyCredentials.emojis || []),
-      displayName: ({ verifyCredentials }) => verifyCredentials.display_name || verifyCredentials.username || '',
-      accessibleName: ({ displayName, emojis, $omitEmojiInDisplayNames }) => {
-        if ($omitEmojiInDisplayNames) {
-          return removeEmoji(displayName, emojis) || displayName
+      href: ({ id }) => (id ? `/accounts/${id}` : '#')
+    },
+    methods: {
+      onClick (e) {
+        let { realm, dialogId, href } = this.get()
+        if (realm === 'dialog') {
+          e.preventDefault()
+          e.stopPropagation()
+          // in dialog mode, we have to close the dialog and then navigate to the profile
+          emit('closeDialog', dialogId)
+          setTimeout(() => { // setTimeout to work around dialog navigation issues
+            goto(href)
+          })
         }
-        return displayName
       }
     }
   }
diff --git a/src/routes/_components/compose/ComposeBox.html b/src/routes/_components/compose/ComposeBox.html
index 20487c0e..ef775a50 100644
--- a/src/routes/_components/compose/ComposeBox.html
+++ b/src/routes/_components/compose/ComposeBox.html
@@ -3,7 +3,7 @@
 {/if}
 <ComposeFileDrop {realm} >
   <div class="{computedClassName} {hideAndFadeIn}">
-    <ComposeAuthor />
+    <ComposeAuthor {realm} {dialogId} />
     {#if contentWarningShown}
       <div class="compose-content-warning-wrapper"
            transition:slide="{duration: 333}">
@@ -120,7 +120,8 @@
       isReply: false,
       autoFocus: false,
       hideBottomBorder: false,
-      hidden: false
+      hidden: false,
+      dialogId: void 0
     }),
     store: () => store,
     computed: {
diff --git a/src/routes/_components/dialog/components/ComposeDialog.html b/src/routes/_components/dialog/components/ComposeDialog.html
index 78415fc3..be98e51e 100644
--- a/src/routes/_components/dialog/components/ComposeDialog.html
+++ b/src/routes/_components/dialog/components/ComposeDialog.html
@@ -4,7 +4,7 @@
   {title}
   background="var(--main-bg)"
 >
-  <ComposeBox realm="dialog" autoFocus="true" />
+  <ComposeBox realm="dialog" autoFocus={true} dialogId={id} />
 </ModalDialog>
 <script>
   import ModalDialog from './ModalDialog.html'
diff --git a/tests/spec/029-back-button-modal.js b/tests/spec/029-back-button-modal.js
index 322f5c43..c057c71a 100644
--- a/tests/spec/029-back-button-modal.js
+++ b/tests/spec/029-back-button-modal.js
@@ -6,7 +6,7 @@ import {
   dialogOptionsOption,
   getNthStatus,
   getNthStatusMediaButton,
-  getNthStatusOptionsButton,
+  getNthStatusOptionsButton, getScrollTop,
   getUrl,
   goBack,
   goForward,
@@ -20,7 +20,7 @@ import {
   visibleModalDialog
 } from '../utils'
 import { loginAsFoobar } from '../roles'
-
+import { Selector as $ } from 'testcafe'
 import { homeTimeline } from '../fixtures'
 
 fixture`029-back-button-modal.js`
@@ -259,3 +259,20 @@ test('History works correctly for nested modal 3', async t => {
   await t
     .expect(getUrl()).contains('/notifications')
 })
+
+test('History and scroll position work correctly for link in compose dialog', async t => {
+  await loginAsFoobar(t)
+  await scrollToStatus(t, 10)
+  await t
+    .expect(getScrollTop()).notEql(0)
+    .click(composeButton)
+    .expect(modalDialog.hasAttribute('aria-hidden')).notOk()
+    .expect(composeModalInput.exists).ok()
+    .click($('.modal-dialog-document .compose-box-display-name'))
+    .expect(getUrl()).contains('/accounts')
+    .expect(getScrollTop()).eql(0)
+  await goBack()
+  await t
+    .expect(getUrl()).eql('http://localhost:4002/')
+    .expect(getScrollTop()).notEql(0)
+})
diff --git a/tests/spec/118-display-name-custom-emoji.js b/tests/spec/118-display-name-custom-emoji.js
index cf953934..725289f3 100644
--- a/tests/spec/118-display-name-custom-emoji.js
+++ b/tests/spec/118-display-name-custom-emoji.js
@@ -1,6 +1,5 @@
 import { loginAsFoobar } from '../roles'
 import {
-  avatarInComposeBox,
   displayNameInComposeBox,
   generalSettingsButton,
   getNthStatus,
@@ -14,6 +13,8 @@ import {
 import { postAs, updateUserDisplayNameAs } from '../serverActions'
 import { Selector as $ } from 'testcafe'
 
+const rainbow = String.fromCodePoint(0x1F308)
+
 fixture`118-display-name-custom-emoji.js`
   .page`http://localhost:4002`
 
@@ -38,59 +39,52 @@ test('Cannot XSS using display name HTML', async t => {
 })
 
 test('Can remove emoji from user display names', async t => {
-  await updateUserDisplayNameAs('foobar', '🌈 foo :blobpats: 🌈')
+  await updateUserDisplayNameAs('foobar', `${rainbow} foo :blobpats: ${rainbow}`)
   await sleep(1000)
   await loginAsFoobar(t)
   await t
-    .expect(displayNameInComposeBox.innerText).match(/🌈\s+foo\s+🌈/)
     .expect($('.compose-box-display-name img').exists).ok()
-    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 foo :blobpats: 🌈')
+    .expect(displayNameInComposeBox.innerText).eql(`${rainbow} foo  ${rainbow}`)
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
     .expect(removeEmojiFromDisplayNamesInput.checked).ok()
     .click(homeNavButton)
-    .expect(displayNameInComposeBox.innerText).eql('foo')
     .expect($('.compose-box-display-name img').exists).notOk()
-    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for foo')
+    .expect(displayNameInComposeBox.innerText).eql('foo')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
     .expect(removeEmojiFromDisplayNamesInput.checked).notOk()
     .click(homeNavButton)
-    .expect(displayNameInComposeBox.innerText).match(/🌈\s+foo\s+🌈/)
     .expect($('.compose-box-display-name img').exists).ok()
-    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 foo :blobpats: 🌈')
+    .expect(displayNameInComposeBox.innerText).eql(`${rainbow} foo  ${rainbow}`)
 })
 
 test('Cannot remove emoji from user display names if result would be empty', async t => {
-  await updateUserDisplayNameAs('foobar', '🌈 :blobpats: 🌈')
+  await updateUserDisplayNameAs('foobar', `${rainbow} :blobpats: ${rainbow}`)
   await sleep(1000)
   await loginAsFoobar(t)
   await t
-    .expect(displayNameInComposeBox.innerText).match(/🌈\s+🌈/)
+    .expect(displayNameInComposeBox.innerText).eql(`${rainbow}  ${rainbow}`)
     .expect($('.compose-box-display-name img').exists).ok()
-    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
     .expect(removeEmojiFromDisplayNamesInput.checked).ok()
     .click(homeNavButton)
-    .expect(displayNameInComposeBox.innerText).match(/🌈\s+🌈/)
+    .expect(displayNameInComposeBox.innerText).eql(`${rainbow}  ${rainbow}`)
     .expect($('.compose-box-display-name img').exists).ok()
-    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
     .click(settingsNavButton)
     .click(generalSettingsButton)
     .click(removeEmojiFromDisplayNamesInput)
     .expect(removeEmojiFromDisplayNamesInput.checked).notOk()
     .click(homeNavButton)
-    .expect(displayNameInComposeBox.innerText).match(/🌈\s+🌈/)
+    .expect(displayNameInComposeBox.innerText).eql(`${rainbow}  ${rainbow}`)
     .expect($('.compose-box-display-name img').exists).ok()
-    .expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
 })
 
 test('Check status aria labels for de-emojified text', async t => {
-  let rainbow = String.fromCodePoint(0x1F308)
   await updateUserDisplayNameAs('foobar', `${rainbow} foo :blobpats: ${rainbow}`)
   await postAs('foobar', 'hey ho lotsa emojos')
   await sleep(1000)
diff --git a/tests/utils.js b/tests/utils.js
index 31f7cbbf..fa94e218 100644
--- a/tests/utils.js
+++ b/tests/utils.js
@@ -189,6 +189,10 @@ export const scrollToTop = exec(() => {
   document.scrollingElement.scrollTop = 0
 })
 
+export const getScrollTop = exec(() => {
+  return document.scrollingElement.scrollTop || 0
+})
+
 export function getNthMediaAltInput (n) {
   return $(`.compose-box .compose-media:nth-child(${n}) .compose-media-alt input`)
 }