Stop opening links in a new tab

frontend/docker-compose.playwright.yml

@@ -1,4 +1,3 @@
-version: "3"
 services:
   playwright:
     build:

frontend/src/components/VLink.vue

@@ -40,20 +40,17 @@
  * Links with `href` starting with `/` are treated as internal links.
  *
  * Internal links use `NuxtLink` component with `to` attribute set to `localePath(href)`
- * External links use `a` element. If `href` does not start with `#`, they are set to
- * open in a new tab.
+ * External links use `a` element, and open in the same tab.
  */
 import { computed, defineComponent } from "vue"
 import { useContext } from "@nuxtjs/composition-api"
 
-import { useAnalytics } from "~/composables/use-analytics"
-
 import { defineEvent } from "~/types/emits"
 
 import VIcon from "~/components/VIcon/VIcon.vue"
 
 type InternalLinkProps = { to: string }
-type ExternalLinkProps = { target: string; rel: string }
+type ExternalLinkProps = { rel: string }
 type DisabledLinkProps = { role: string }
 type LinkProps =
   | InternalLinkProps
@@ -128,20 +125,20 @@ export default defineComponent({
           return null
         } else {
           // External link should open in a new tab
-          return { target: "_blank", rel: "noopener noreferrer" }
+          return { rel: "noopener noreferrer" }
         }
       }
       // if href is undefined, return props that make the link disabled
       return { role: "link" }
     })
 
-    const { sendCustomEvent } = useAnalytics()
+    const { $sendCustomEvent } = useContext()
 
     const handleExternalClick = () => {
       if (!checkHref(props) || !props.sendExternalLinkClickEvent) {
         return
       }
-      sendCustomEvent("EXTERNAL_LINK_CLICK", {
+      $sendCustomEvent("EXTERNAL_LINK_CLICK", {
         url: props.href,
       })
     }

frontend/src/utils/attribution-html.ts

@@ -122,7 +122,7 @@ const licenseElementImg = (licenseElement: LicenseElement): string => {
  * @returns the HTML markup of the `<a>` tag
  */
 const extLink = (href: string, text: string) =>
-  h("a", { target: "_blank", rel: "noopener noreferrer", href }, [text])
+  h("a", { rel: "noopener noreferrer", href }, [text])
 
 /* Interfaces */
 

frontend/test/playwright/e2e/audio-detail.spec.ts

@@ -4,7 +4,10 @@ import {
   collectAnalyticsEvents,
   expectEventPayloadToMatch,
 } from "~~/test/playwright/utils/analytics"
-import { preparePageForTests } from "~~/test/playwright/utils/navigation"
+import {
+  openAndCloseExternalLink,
+  preparePageForTests,
+} from "~~/test/playwright/utils/navigation"
 import { t } from "~~/test/playwright/utils/i18n"
 import { getH1 } from "~~/test/playwright/utils/components"
 
@@ -42,12 +45,9 @@ test("sends GET_MEDIA event on CTA button click", async ({ context, page }) => {
   const analyticsEvents = collectAnalyticsEvents(context)
 
   await goToCustomAudioPage(page)
-
-  const pagePromise = context.waitForEvent("page")
-  await page.getByRole("link", { name: /get this audio/i }).click()
-
-  const newPage = await pagePromise
-  newPage.close()
+  await openAndCloseExternalLink(page, {
+    name: new RegExp(t("audioDetails.weblink"), "i"),
+  })
 
   const getMediaEvent = analyticsEvents.find((event) => event.n === "GET_MEDIA")
 

frontend/test/playwright/e2e/external-sources.spec.ts

@@ -2,6 +2,7 @@ import { expect, test } from "@playwright/test"
 
 import {
   goToSearchTerm,
+  openAndCloseExternalLink,
   preparePageForTests,
 } from "~~/test/playwright/utils/navigation"
 
@@ -44,8 +45,6 @@ test.describe("analytics", () => {
     page,
     context,
   }) => {
-    const pagePromise = page.context().waitForEvent("page")
-
     const events = collectAnalyticsEvents(context)
 
     await goToSearchTerm(page, "cat", { searchType: "image", mode: "SSR" })
@@ -55,10 +54,7 @@ test.describe("analytics", () => {
         name: new RegExp(t("externalSources.form.supportedTitleSm"), "i"),
       })
       .click()
-    await page.getByRole("link", { name: "Centre for Ageing Better" }).click()
-
-    const newPage = await pagePromise
-    await newPage.close()
+    await openAndCloseExternalLink(page, { name: "Centre for Ageing Better" })
 
     const selectEvent = events.find(
       (event) => event.n === "SELECT_EXTERNAL_SOURCE"

frontend/test/playwright/e2e/header-internal.spec.ts

@@ -2,6 +2,7 @@ import { expect, Page, test } from "@playwright/test"
 
 import {
   isDialogOpen,
+  openAndCloseExternalLink,
   preparePageForTests,
   scrollToBottom,
 } from "~~/test/playwright/utils/navigation"
@@ -63,21 +64,17 @@ test.describe("Header internal", () => {
       expect(scrollPosition).toBeGreaterThan(100)
     })
 
-    test("the modal opens an external link in a new window and it closes the modal", async ({
+    test("the modal opens an external link in the same window and the modal is closed on back click", async ({
       page,
     }) => {
       await page.goto("/about")
       await scrollToBottom(page)
       await clickMenuButton(page)
 
-      // Open the external link in a new tab, close the tab
-      const [popup] = await Promise.all([
-        page.waitForEvent("popup"),
-        page.locator('div[role="dialog"] >> text=API').click(),
-      ])
-      await popup.close()
-      // If we want the modal to stay open, we'll need to change this to `true`,
-      // and implement the change
+      await openAndCloseExternalLink(page, {
+        locator: page.locator('div[role="dialog"] >> text=API'),
+      })
+
       expect(await isDialogOpen(page)).toBe(false)
     })
 

frontend/test/playwright/e2e/report-media.spec.ts

@@ -13,7 +13,7 @@ import {
 } from "~~/test/playwright/utils/analytics"
 
 import { supportedMediaTypes } from "~/constants/media"
-import { ReportReason } from "~/constants/content-report"
+import type { ReportReason } from "~/constants/content-report"
 
 test.describe.configure({ mode: "parallel" })
 
@@ -57,13 +57,12 @@ const submitDmcaReport = async (page: Page, context: BrowserContext) => {
     })
   })
   await page.click('text="Infringes copyright"')
-  const [newPage] = await Promise.all([
-    context.waitForEvent("page"),
-    await page.click('text="Open form"'), // Opens a new tab
-  ])
-  await newPage.waitForLoadState()
+
+  await page.click('text="Open form"')
+  await page.waitForURL(/forms/)
+
   // Return the beginning of the url, without parameters
-  return newPage.url().split("/forms/")[0] + "/forms/"
+  return page.url().split("/forms/")[0] + "/forms/"
 }
 
 // todo: Test a sensitive report with the optional description field

frontend/test/playwright/e2e/translation-banner.spec.ts

@@ -15,11 +15,8 @@ test.describe("translation banner", () => {
     await page.goto(localeSearchPath)
     await expect(page.locator("text= 100")).toBeVisible({ timeout: 500 })
 
-    const [page1] = await Promise.all([
-      page.waitForEvent("popup"),
-      page.click("text=contributing a translation"),
-    ])
-    await expect(page1).toHaveURL(
+    await page.click("text=contributing a translation")
+    await expect(page).toHaveURL(
       `https://translate.wordpress.org/projects/meta/openverse/${locale}/default/`
     )
   })

frontend/test/playwright/utils/navigation.ts

@@ -448,3 +448,20 @@ export const skipToContent = async (page: Page) => {
   // Skip to content
   await page.keyboard.press(keycodes.Enter)
 }
+
+export const openAndCloseExternalLink = async (
+  page: Page,
+  { name, locator }: { name?: string | RegExp | undefined; locator?: Locator }
+) => {
+  if (!name && !locator) {
+    throw new Error("Either name or locator must be provided")
+  }
+  const externalLink: Locator = locator ?? page.getByRole("link", { name })
+  const externalLinkUrl = await externalLink.getAttribute("href")
+  if (!externalLinkUrl) {
+    throw new Error("External link URL not found")
+  }
+  await externalLink.click()
+
+  await page.goBack()
+}

frontend/test/unit/specs/components/v-link.spec.js

@@ -3,22 +3,19 @@ import Vue from "vue"
 
 import { render } from "~~/test/unit/test-utils/render"
 
-import { useAnalytics } from "~/composables/use-analytics"
-
 import VLink from "~/components/VLink.vue"
 
-jest.mock("~/composables/use-analytics", () => ({
-  useAnalytics: jest.fn(),
+jest.mock("@nuxtjs/composition-api", () => ({
+  useContext: () => ({
+    $sendCustomEvent: jest.fn(),
+  }),
 }))
 
 describe("VLink", () => {
-  useAnalytics.mockImplementation(() => ({
-    sendCustomEvent: jest.fn(),
-  }))
   it.each`
-    href                        | target      | rel
-    ${"/about"}                 | ${null}     | ${null}
-    ${"http://localhost:8443/"} | ${"_blank"} | ${"noopener noreferrer"}
+    href                        | target  | rel
+    ${"/about"}                 | ${null} | ${null}
+    ${"http://localhost:8443/"} | ${null} | ${"noopener noreferrer"}
   `(
     "Creates a correct link component based on href",
     ({ href, target, rel }) => {