fix(html): drop the blanket link target, and mark only external links

AGENTS.md

@@ -192,10 +192,11 @@ Dispatch `release.yml` against main, publish the draft that appears —
   construct. Nothing is passed through as live markup, which is why an svg goes
   out as a data url rather than inlined ([`svg/AGENTS.md`](src/odr/internal/svg/AGENTS.md))
   and why the rendered page needs no sanitiser. A link target goes through one
-  allowlist, `html::is_safe_uri` in `html/common.cpp`, called by both a PDF
-  `/URI` action and `html/document_element.cpp::translate_link`; a refused
-  target loses its `href` and keeps its text. A third writer of an `href` calls
-  it too.
+  classifier, `html::uri_kind` in `html/common.cpp`, called by both a PDF
+  `/URI` action and `html/document_element.cpp::translate_link`: a refused
+  target loses its `href` and keeps its text, an external one gets
+  `target="_blank"`, a relative one no target. No view declares a document-wide
+  `<base target>`. A third writer of an `href` calls it too.
 - **Public API**: value semantics; immutable handles; iterators only for immutable
   traversal (`docs/design/README.md`).
 - **Byte parsing**: read POD structs via `util::byte_stream::read`; assumes host

CHANGELOG.md

@@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- No view declares `<base target="_blank">` any more. A link back into what
+  serves the page — an archive entry, a PDF `#pN` anchor, a relative hyperlink —
+  navigates in place; only a link that leaves the page carries
+  `target="_blank" rel="noopener noreferrer"`.
+
 - A document hyperlink renders without an `href` unless its target is `http`,
   `https`, `mailto`, `ftp`, `ftps`, `tel` or a relative reference — the
   allowlist a PDF `/URI` action already went through. `Link::href()` is

src/odr/internal/html/common.cpp

@@ -282,19 +282,21 @@ std::string html::escape_attribute(std::string value) {
   return value;
 }
 
-bool html::is_safe_uri(const std::string_view uri) {
+html::UriKind html::uri_kind(const std::string_view uri) {
   std::string scheme;
   for (const char ch : uri) {
     const auto c = static_cast<unsigned char>(ch);
     if (ch == ':') {
       static constexpr std::array<std::string_view, 6> allowed = {
           "http", "https", "mailto", "ftp", "ftps", "tel"};
-      return std::ranges::any_of(allowed, [&scheme](const std::string_view s) {
-        return util::string::equals_ignore_case(scheme, s);
-      });
+      const bool navigable =
+          std::ranges::any_of(allowed, [&scheme](const std::string_view s) {
+            return util::string::equals_ignore_case(scheme, s);
+          });
+      return navigable ? UriKind::external : UriKind::refused;
     }
     if (ch == '/' || ch == '?' || ch == '#') {
-      return true; // path/query/fragment reached first -> relative reference
+      return UriKind::relative; // a path/query/fragment came first
     }
     if (c <= 0x20) {
       continue; // browsers strip embedded whitespace/control bytes
@@ -303,9 +305,15 @@ bool html::is_safe_uri(const std::string_view uri) {
       scheme.push_back(ch);
       continue;
     }
-    return true; // not a valid scheme character -> relative reference
+    return UriKind::relative; // not a scheme character
   }
-  return true; // no ':' -> relative reference
+  return UriKind::relative; // no ':'
+}
+
+std::string_view html::link_target_attributes(const UriKind kind) {
+  return kind == UriKind::external
+             ? R"(target="_blank" rel="noopener noreferrer")"
+             : std::string_view();
 }
 
 std::string

src/odr/internal/html/common.hpp

@@ -84,10 +84,24 @@ std::string escape_text(std::string text);
 /// `<`, `>`). Unlike `escape_text`, it leaves leading/trailing spaces intact.
 std::string escape_attribute(std::string value);
 
-/// Whether a target is safe to emit as an `href`: the navigable schemes plus
-/// scheme-less (relative) references. Embedded whitespace and control bytes are
-/// skipped while reading the scheme, as browsers strip them before dispatch.
-[[nodiscard]] bool is_safe_uri(std::string_view uri);
+/// What a target is, as an `href` would be dispatched. Whitespace and control
+/// bytes are skipped while reading the scheme, as browsers strip them first.
+enum class UriKind {
+  relative, ///< no scheme
+  external, ///< a navigable scheme
+  refused,  ///< `javascript:` and kin
+};
+
+[[nodiscard]] UriKind uri_kind(std::string_view uri);
+
+/// Safe to emit as an `href`.
+[[nodiscard]] inline bool is_safe_uri(const std::string_view uri) {
+  return uri_kind(uri) != UriKind::refused;
+}
+
+/// The `<a>` attributes for @p kind, unprefixed; empty but for
+/// @ref UriKind::external.
+[[nodiscard]] std::string_view link_target_attributes(UriKind kind);
 
 std::string color(const Color &color);
 

src/odr/internal/html/document.cpp

@@ -131,7 +131,6 @@ void front(const Document &document, const WritingState &state,
   out.write_begin();
   out.write_header_begin();
   out.write_header_charset("UTF-8");
-  out.write_header_target("_blank");
   out.write_header_title(
       document.document_type() == DocumentType::spreadsheet && !name.empty()
           ? escape_text(name)

src/odr/internal/html/document_element.cpp

@@ -408,15 +408,23 @@ void html::translate_span(const Element &element, const WritingState &state) {
 void html::translate_link(const Element &element, const WritingState &state) {
   const Link link = element.as_link();
   const std::string href = link.href();
+  const UriKind kind = uri_kind(href);
 
+  // A refused target loses the attribute, not the element.
   HtmlAttributesVector attributes;
-  if (is_safe_uri(href)) {
+  if (kind != UriKind::refused) {
     attributes.emplace_back("href", escape_attribute(href));
   }
 
-  state.out().write_element_begin(
-      "a", HtmlElementOptions().set_inline(true).set_attributes(
-               std::move(attributes)));
+  HtmlElementOptions options =
+      HtmlElementOptions().set_inline(true).set_attributes(
+          std::move(attributes));
+  if (const std::string_view target = link_target_attributes(kind);
+      !target.empty()) {
+    options.set_extra(std::string(target));
+  }
+
+  state.out().write_element_begin("a", options);
   translate_children(link.children(), state);
   state.out().write_element_end("a");
 }

src/odr/internal/html/filesystem.cpp

@@ -185,7 +185,6 @@ class HtmlServiceImpl final : public HtmlService {
 
     out.write_header_begin();
     out.write_header_charset("UTF-8");
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), false);
     write_zoom_style(out, config(), WidthFit::none, {});

src/odr/internal/html/font_file.cpp

@@ -74,7 +74,6 @@ class HtmlServiceImpl final : public HtmlService {
     out.write_begin();
     out.write_header_begin();
     out.write_header_charset("UTF-8");
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), false);
     write_content_margin_style(out, config());

src/odr/internal/html/html_writer.cpp

@@ -176,14 +176,6 @@ void HtmlWriter::write_header_viewport(const std::string &viewport) {
   write_header_meta("viewport", viewport);
 }
 
-void HtmlWriter::write_header_target(const std::string &target) {
-  write_new_line();
-
-  out() << "<base target=\"";
-  out() << target;
-  out() << "\"/>";
-}
-
 void HtmlWriter::write_header_charset(const std::string &charset) {
   write_new_line();
 

src/odr/internal/html/html_writer.hpp

@@ -60,7 +60,6 @@ class HtmlWriter {
   void write_header_title(const std::string &title);
   void write_header_meta(const std::string &name, const std::string &content);
   void write_header_viewport(const std::string &viewport);
-  void write_header_target(const std::string &target);
   void write_header_charset(const std::string &charset);
   /// @p media, when given, gates the stylesheet on that media query.
   void write_header_style(const std::string &href, std::string_view media = {});

src/odr/internal/html/image_file.cpp

@@ -115,7 +115,6 @@ class HtmlServiceImpl final : public HtmlService {
     out.write_begin();
     out.write_header_begin();
     out.write_header_charset("UTF-8");
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), true);
     // An image has no layout width to preserve, so css alone fits it, framed

src/odr/internal/html/media_file.cpp

@@ -166,7 +166,6 @@ class HtmlServiceImpl final : public HtmlService {
     out.write_begin();
     out.write_header_begin();
     out.write_header_charset("UTF-8");
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), false);
     write_media_style(state);

src/odr/internal/html/pdf_file.cpp

@@ -257,11 +257,12 @@ std::vector<LinkOut> collect_page_links(const pdf::Page &page,
 void write_page_links(HtmlWriter &out, const std::vector<LinkOut> &links) {
   for (const LinkOut &link : links) {
     std::ostringstream a;
-    // Internal `#pN` links must override the document's `<base
-    // target="_blank">` or they open a new copy instead of scrolling.
-    a << "<a class=\"lk\" href=\"" << link.href << '"'
-      << (link.internal ? " target=\"_self\"" : "")
-      << " style=\"left:" << round2(link.left) << "pt;top:" << round2(link.top)
+    // A `#pN` link scrolls this page; a `/URI` action leaves it.
+    a << "<a class=\"lk\" href=\"" << link.href << '"';
+    if (!link.internal) {
+      a << ' ' << link_target_attributes(UriKind::external);
+    }
+    a << " style=\"left:" << round2(link.left) << "pt;top:" << round2(link.top)
       << "pt;width:" << round2(link.width)
       << "pt;height:" << round2(link.height) << "pt\"></a>";
     out.write_raw(std::move(a).str());
@@ -2672,7 +2673,6 @@ class HtmlServiceImpl final : public HtmlService {
     out.write_begin();
     out.write_header_begin();
     out.write_header_charset("UTF-8");
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), true);
     write_zoom_style(out, config(), width_fit(config(), true), content);

src/odr/internal/html/text_file.cpp

@@ -96,7 +96,6 @@ class HtmlServiceImpl final : public HtmlService {
     out.write_header_begin();
 
     out.write_header_charset(charset);
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), false);
     write_zoom_style(out, config(), WidthFit::none, {});

src/odr/internal/html/xml_file.cpp

@@ -256,7 +256,6 @@ class HtmlServiceImpl final : public HtmlService {
     out.write_header_begin();
 
     out.write_header_charset("UTF-8");
-    out.write_header_target("_blank");
     out.write_header_title("odr");
     write_viewport_meta(out, config(), false);
     write_zoom_style(out, config(), WidthFit::none, {});

test/data.cmake

@@ -17,9 +17,9 @@ odr_test_data(
 odr_test_data(
         PATH "reference-output/odr-public"
         URL "https://github.com/opendocument-app/OpenDocument.test.output.git"
-        REVISION "55f56ffe133c9140fe88f6352eb634ccd32e4173")
+        REVISION "54fe51a0e28d95287fa3ca5d1112825706511067")
 
 odr_test_data(
         PATH "reference-output/odr-private"
         URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git"
-        REVISION "d7274fe8e03b43f3a2857883478a7a01e2d6b7c5")
+        REVISION "420e0669f520aba75be4eb966f306ef53829dccd")

test/src/html_test.cpp

@@ -628,17 +628,51 @@ TEST(html, a_link_the_page_must_not_navigate_to_loses_its_href) {
   }
 }
 
+// #730
 TEST(html, a_link_that_is_navigable_keeps_its_href) {
-  EXPECT_NE(
-      render_markdown("[a](https://x.example/?u=1&v=2)\n")
-          .find(R"(<a href="https://x.example/?u=1&amp;v=2"><x-s>a</x-s></a>)"),
-      std::string::npos);
-  EXPECT_NE(render_markdown("[a](mailto:someone@x.example)\n")
-                .find(R"(<a href="mailto:someone@x.example"><x-s>a</x-s></a>)"),
+  const std::string away = R"( target="_blank" rel="noopener noreferrer")";
+
+  EXPECT_NE(render_markdown("[a](https://x.example/?u=1&amp;v=2)\n")
+                .find(R"(<a href="https://x.example/?u=1&amp;v=2")" + away +
+                      "><x-s>a</x-s></a>"),
             std::string::npos);
-  EXPECT_NE(render_markdown("[a](#bookmark)\n")
-                .find(R"(<a href="#bookmark"><x-s>a</x-s></a>)"),
+  EXPECT_NE(render_markdown("[a](mailto:someone@x.example)\n")
+                .find(R"(<a href="mailto:someone@x.example")" + away +
+                      "><x-s>a</x-s></a>"),
             std::string::npos);
+
+  for (const std::string_view target :
+       {"#bookmark", "other.html", "a/b.html"}) {
+    const std::string page =
+        render_markdown("[a](" + std::string(target) + ")\n");
+    EXPECT_NE(page.find(R"(<a href=")" + std::string(target) +
+                        R"("><x-s>a</x-s></a>)"),
+              std::string::npos)
+        << target;
+    EXPECT_EQ(page.find("_blank"), std::string::npos) << target;
+  }
+}
+
+// #730
+TEST(html, no_view_declares_a_document_wide_link_target) {
+  const auto render = [](const DecodedFile &file) {
+    std::ostringstream out;
+    html::translate(file, HtmlConfig()).list_views().at(0).write_html(out);
+    return std::move(out).str();
+  };
+
+  const std::array views{
+      render(DecodedFile(File::from_memory("a,b\n1,2\n"),
+                         FileType::comma_separated_values)),
+      render(DecodedFile(File::from_memory("<a><b>c</b></a>"), FileType::xml)),
+      render(DecodedFile(File::from_memory("plain text"), FileType::text_file)),
+      render(DecodedFile(File::from_memory("[a](https://x.example)\n"),
+                         FileType::markdown)),
+  };
+
+  for (const std::string &view : views) {
+    EXPECT_EQ(view.find("<base"), std::string::npos);
+  }
 }
 
 // #740: a sheet that ran into a limit used to end without saying so.

test/src/internal/pdf/pdf_file.cpp

@@ -195,11 +195,10 @@ TEST(PdfFile, file_meta_without_info) {
   EXPECT_FALSE(meta.author.has_value());
 }
 
-// `/Link` annotations render as `<a>` overlays: a `/URI` action → external href
-// (with `&` attr-escaped), a direct `/Dest` and a named `/GoTo` → internal
-// `#pN` anchors that carry `target="_self"` (overriding `<base
-// target="_blank">`); each page div carries a matching `id`. An active-scheme
-// `/URI` is dropped.
+// `/Link` annotations render as `<a>` overlays: a `/URI` action → an external
+// href (`&` attr-escaped, opening away from the page), a direct `/Dest` and a
+// named `/GoTo` → internal `#pN` anchors with a matching page div `id`. An
+// active-scheme `/URI` is dropped.
 TEST(PdfFile, link_annotations_render_as_anchors) {
   const std::string pdf = link_annotations_mini_pdf();
   for (const PdfTextMode mode :
@@ -209,11 +208,13 @@ TEST(PdfFile, link_annotations_render_as_anchors) {
         << "mode " << static_cast<int>(mode);
     EXPECT_TRUE(contains(html, R"(id="p3")"))
         << "mode " << static_cast<int>(mode);
-    EXPECT_TRUE(contains(html, R"(href="http://example.com/?a=1&amp;b=2")"))
+    EXPECT_TRUE(contains(
+        html,
+        R"(href="http://example.com/?a=1&amp;b=2" target="_blank" rel="noopener noreferrer")"))
         << "mode " << static_cast<int>(mode);
-    EXPECT_TRUE(contains(html, R"(href="#p2" target="_self")"))
+    EXPECT_TRUE(contains(html, R"(href="#p2" style=)"))
         << "mode " << static_cast<int>(mode);
-    EXPECT_TRUE(contains(html, R"(href="#p3" target="_self")"))
+    EXPECT_TRUE(contains(html, R"(href="#p3" style=)"))
         << "mode " << static_cast<int>(mode);
     // The `javascript:` action is not emitted as a link.
     EXPECT_FALSE(contains(html, "javascript:alert"))
@@ -267,8 +268,8 @@ TEST(PdfFile, page_views_link_between_page_files) {
                                        /*path=*/"page0.html");
   EXPECT_TRUE(contains(html, R"(id="p1")"));
   EXPECT_FALSE(contains(html, R"(id="p2")"));
-  EXPECT_TRUE(contains(html, R"(href="page1.html" target="_self")"));
-  EXPECT_TRUE(contains(html, R"(href="page2.html" target="_self")"));
+  EXPECT_TRUE(contains(html, R"(href="page1.html" style=)"));
+  EXPECT_TRUE(contains(html, R"(href="page2.html" style=)"));
 
   const std::string page3 = render_html(pdf, PdfTextMode::dual_layer,
                                         /*path=*/"page2.html");
@@ -303,7 +304,7 @@ TEST(PdfFile, page_views_nested_output_pattern_links_relatively) {
     const HtmlService service = make_service(pdf, config);
     EXPECT_EQ(service.list_views().at(1).path(), "pages/page0.html");
     const std::string html = render_path(service, "pages/page0.html");
-    EXPECT_TRUE(contains(html, R"(href="page1.html" target="_self")"));
+    EXPECT_TRUE(contains(html, R"(href="page1.html" style=)"));
     EXPECT_FALSE(contains(html, R"(href="pages/page1.html")"));
   }
 
@@ -313,8 +314,8 @@ TEST(PdfFile, page_views_nested_output_pattern_links_relatively) {
     config.page_output_file_name = "p{index}/index.html";
     const HtmlService service = make_service(pdf, config);
     const std::string html = render_path(service, "p0/index.html");
-    EXPECT_TRUE(contains(html, R"(href="../p1/index.html" target="_self")"));
-    EXPECT_TRUE(contains(html, R"(href="../p2/index.html" target="_self")"));
+    EXPECT_TRUE(contains(html, R"(href="../p1/index.html" style=)"));
+    EXPECT_TRUE(contains(html, R"(href="../p2/index.html" style=)"));
   }
 }