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
This commit is contained in:
Nolan Lawson 2019-04-20 09:12:30 -07:00 committed by GitHub
parent 2ce2453d8f
commit 1712081f0b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 59 additions and 32 deletions

View File

@ -1,16 +1,20 @@
<a {href} <a {href}
rel="prefetch" rel="prefetch"
class="compose-box-avatar {loaded ? 'loaded' : 'not-loaded'}" class="compose-box-avatar {loaded ? 'loaded' : 'not-loaded'}"
aria-hidden={!loaded} aria-hidden={true}
aria-label="Profile for {accessibleName}"> on:click="onClick(event)"
>
<Avatar account={verifyCredentials} size="small"/> <Avatar account={verifyCredentials} size="small"/>
</a> </a>
<a class="compose-box-display-name {loaded ? 'loaded' : 'not-loaded'}" <a class="compose-box-display-name {loaded ? 'loaded' : 'not-loaded'}"
{href} {href}
aria-hidden={!loaded} aria-hidden={!loaded}
rel="prefetch"> rel="prefetch"
on:click="onClick(event)"
>
<AccountDisplayName account={verifyCredentials} /> <AccountDisplayName account={verifyCredentials} />
</a> </a>
<span class="compose-box-handle {loaded ? 'loaded' : 'not-loaded'}" <span class="compose-box-handle {loaded ? 'loaded' : 'not-loaded'}"
aria-hidden={!loaded} > aria-hidden={!loaded} >
{'@' + verifyCredentials.acct} {'@' + verifyCredentials.acct}
@ -64,8 +68,9 @@
import Avatar from '../Avatar.html' import Avatar from '../Avatar.html'
import { store } from '../../_store/store' import { store } from '../../_store/store'
import AccountDisplayName from '../profile/AccountDisplayName.html' import AccountDisplayName from '../profile/AccountDisplayName.html'
import { removeEmoji } from '../../_utils/removeEmoji'
import { ONE_TRANSPARENT_PIXEL } from '../../_static/media' import { ONE_TRANSPARENT_PIXEL } from '../../_static/media'
import { emit } from '../../_utils/eventBus'
import { goto } from '../../../../__sapper__/client'
export default { export default {
components: { components: {
@ -86,14 +91,20 @@
} }
}, },
id: ({ verifyCredentials }) => (verifyCredentials && verifyCredentials.id), id: ({ verifyCredentials }) => (verifyCredentials && verifyCredentials.id),
href: ({ id }) => (id ? `/accounts/${id}` : '#'), href: ({ id }) => (id ? `/accounts/${id}` : '#')
emojis: ({ verifyCredentials }) => (verifyCredentials.emojis || []), },
displayName: ({ verifyCredentials }) => verifyCredentials.display_name || verifyCredentials.username || '', methods: {
accessibleName: ({ displayName, emojis, $omitEmojiInDisplayNames }) => { onClick (e) {
if ($omitEmojiInDisplayNames) { let { realm, dialogId, href } = this.get()
return removeEmoji(displayName, emojis) || displayName 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
} }
} }
} }

View File

@ -3,7 +3,7 @@
{/if} {/if}
<ComposeFileDrop {realm} > <ComposeFileDrop {realm} >
<div class="{computedClassName} {hideAndFadeIn}"> <div class="{computedClassName} {hideAndFadeIn}">
<ComposeAuthor /> <ComposeAuthor {realm} {dialogId} />
{#if contentWarningShown} {#if contentWarningShown}
<div class="compose-content-warning-wrapper" <div class="compose-content-warning-wrapper"
transition:slide="{duration: 333}"> transition:slide="{duration: 333}">
@ -120,7 +120,8 @@
isReply: false, isReply: false,
autoFocus: false, autoFocus: false,
hideBottomBorder: false, hideBottomBorder: false,
hidden: false hidden: false,
dialogId: void 0
}), }),
store: () => store, store: () => store,
computed: { computed: {

View File

@ -4,7 +4,7 @@
{title} {title}
background="var(--main-bg)" background="var(--main-bg)"
> >
<ComposeBox realm="dialog" autoFocus="true" /> <ComposeBox realm="dialog" autoFocus={true} dialogId={id} />
</ModalDialog> </ModalDialog>
<script> <script>
import ModalDialog from './ModalDialog.html' import ModalDialog from './ModalDialog.html'

View File

@ -6,7 +6,7 @@ import {
dialogOptionsOption, dialogOptionsOption,
getNthStatus, getNthStatus,
getNthStatusMediaButton, getNthStatusMediaButton,
getNthStatusOptionsButton, getNthStatusOptionsButton, getScrollTop,
getUrl, getUrl,
goBack, goBack,
goForward, goForward,
@ -20,7 +20,7 @@ import {
visibleModalDialog visibleModalDialog
} from '../utils' } from '../utils'
import { loginAsFoobar } from '../roles' import { loginAsFoobar } from '../roles'
import { Selector as $ } from 'testcafe'
import { homeTimeline } from '../fixtures' import { homeTimeline } from '../fixtures'
fixture`029-back-button-modal.js` fixture`029-back-button-modal.js`
@ -259,3 +259,20 @@ test('History works correctly for nested modal 3', async t => {
await t await t
.expect(getUrl()).contains('/notifications') .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)
})

View File

@ -1,6 +1,5 @@
import { loginAsFoobar } from '../roles' import { loginAsFoobar } from '../roles'
import { import {
avatarInComposeBox,
displayNameInComposeBox, displayNameInComposeBox,
generalSettingsButton, generalSettingsButton,
getNthStatus, getNthStatus,
@ -14,6 +13,8 @@ import {
import { postAs, updateUserDisplayNameAs } from '../serverActions' import { postAs, updateUserDisplayNameAs } from '../serverActions'
import { Selector as $ } from 'testcafe' import { Selector as $ } from 'testcafe'
const rainbow = String.fromCodePoint(0x1F308)
fixture`118-display-name-custom-emoji.js` fixture`118-display-name-custom-emoji.js`
.page`http://localhost:4002` .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 => { 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 sleep(1000)
await loginAsFoobar(t) await loginAsFoobar(t)
await t await t
.expect(displayNameInComposeBox.innerText).match(/🌈\s+foo\s+🌈/)
.expect($('.compose-box-display-name img').exists).ok() .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(settingsNavButton)
.click(generalSettingsButton) .click(generalSettingsButton)
.click(removeEmojiFromDisplayNamesInput) .click(removeEmojiFromDisplayNamesInput)
.expect(removeEmojiFromDisplayNamesInput.checked).ok() .expect(removeEmojiFromDisplayNamesInput.checked).ok()
.click(homeNavButton) .click(homeNavButton)
.expect(displayNameInComposeBox.innerText).eql('foo')
.expect($('.compose-box-display-name img').exists).notOk() .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(settingsNavButton)
.click(generalSettingsButton) .click(generalSettingsButton)
.click(removeEmojiFromDisplayNamesInput) .click(removeEmojiFromDisplayNamesInput)
.expect(removeEmojiFromDisplayNamesInput.checked).notOk() .expect(removeEmojiFromDisplayNamesInput.checked).notOk()
.click(homeNavButton) .click(homeNavButton)
.expect(displayNameInComposeBox.innerText).match(/🌈\s+foo\s+🌈/)
.expect($('.compose-box-display-name img').exists).ok() .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 => { 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 sleep(1000)
await loginAsFoobar(t) await loginAsFoobar(t)
await t await t
.expect(displayNameInComposeBox.innerText).match(/🌈\s+🌈/) .expect(displayNameInComposeBox.innerText).eql(`${rainbow} ${rainbow}`)
.expect($('.compose-box-display-name img').exists).ok() .expect($('.compose-box-display-name img').exists).ok()
.expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
.click(settingsNavButton) .click(settingsNavButton)
.click(generalSettingsButton) .click(generalSettingsButton)
.click(removeEmojiFromDisplayNamesInput) .click(removeEmojiFromDisplayNamesInput)
.expect(removeEmojiFromDisplayNamesInput.checked).ok() .expect(removeEmojiFromDisplayNamesInput.checked).ok()
.click(homeNavButton) .click(homeNavButton)
.expect(displayNameInComposeBox.innerText).match(/🌈\s+🌈/) .expect(displayNameInComposeBox.innerText).eql(`${rainbow} ${rainbow}`)
.expect($('.compose-box-display-name img').exists).ok() .expect($('.compose-box-display-name img').exists).ok()
.expect(avatarInComposeBox.getAttribute('aria-label')).eql('Profile for 🌈 :blobpats: 🌈')
.click(settingsNavButton) .click(settingsNavButton)
.click(generalSettingsButton) .click(generalSettingsButton)
.click(removeEmojiFromDisplayNamesInput) .click(removeEmojiFromDisplayNamesInput)
.expect(removeEmojiFromDisplayNamesInput.checked).notOk() .expect(removeEmojiFromDisplayNamesInput.checked).notOk()
.click(homeNavButton) .click(homeNavButton)
.expect(displayNameInComposeBox.innerText).match(/🌈\s+🌈/) .expect(displayNameInComposeBox.innerText).eql(`${rainbow} ${rainbow}`)
.expect($('.compose-box-display-name img').exists).ok() .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 => { test('Check status aria labels for de-emojified text', async t => {
let rainbow = String.fromCodePoint(0x1F308)
await updateUserDisplayNameAs('foobar', `${rainbow} foo :blobpats: ${rainbow}`) await updateUserDisplayNameAs('foobar', `${rainbow} foo :blobpats: ${rainbow}`)
await postAs('foobar', 'hey ho lotsa emojos') await postAs('foobar', 'hey ho lotsa emojos')
await sleep(1000) await sleep(1000)

View File

@ -189,6 +189,10 @@ export const scrollToTop = exec(() => {
document.scrollingElement.scrollTop = 0 document.scrollingElement.scrollTop = 0
}) })
export const getScrollTop = exec(() => {
return document.scrollingElement.scrollTop || 0
})
export function getNthMediaAltInput (n) { export function getNthMediaAltInput (n) {
return $(`.compose-box .compose-media:nth-child(${n}) .compose-media-alt input`) return $(`.compose-box .compose-media:nth-child(${n}) .compose-media-alt input`)
} }