Don't link items if container is already linked (#5732)

16 files changed, 185 insertions, 98 deletions

diff --git a/crates/typst-layout/src/flow/collect.rs b/crates/typst-layout/src/flow/collect.rs
index 12cfa152..76d7b743 100644
--- a/crates/typst-layout/src/flow/collect.rs
+++ b/crates/typst-layout/src/flow/collect.rs
@@ -22,6 +22,7 @@ use typst_library::text::TextElem;
 use typst_library::World;
 
 use super::{layout_multi_block, layout_single_block};
+use crate::modifiers::layout_and_modify;
 
 /// Collects all elements of the flow into prepared children. These are much
 /// simpler to handle than the raw elements.
@@ -377,8 +378,9 @@ fn layout_single_impl(
         route: Route::extend(route),
     };
 
-    layout_single_block(elem, &mut engine, locator, styles, region)
-        .map(|frame| frame.post_processed(styles))
+    layout_and_modify(styles, |styles| {
+        layout_single_block(elem, &mut engine, locator, styles, region)
+    })
 }
 
 /// A child that encapsulates a prepared breakable block.
@@ -473,11 +475,8 @@ fn layout_multi_impl(
         route: Route::extend(route),
     };
 
-    layout_multi_block(elem, &mut engine, locator, styles, regions).map(|mut fragment| {
-        for frame in &mut fragment {
-            frame.post_process(styles);
-        }
-        fragment
+    layout_and_modify(styles, |styles| {
+        layout_multi_block(elem, &mut engine, locator, styles, regions)
     })
 }
 
@@ -579,20 +578,23 @@ impl PlacedChild<'_> {
         self.cell.get_or_init(base, |base| {
             let align = self.alignment.unwrap_or_else(|| Alignment::CENTER);
             let aligned = AlignElem::set_alignment(align).wrap();
-
-            let mut frame = crate::layout_frame(
-                engine,
-                &self.elem.body,
-                self.locator.relayout(),
-                self.styles.chain(&aligned),
-                Region::new(base, Axes::splat(false)),
-            )?;
+            let styles = self.styles.chain(&aligned);
+
+            let mut frame = layout_and_modify(styles, |styles| {
+                crate::layout_frame(
+                    engine,
+                    &self.elem.body,
+                    self.locator.relayout(),
+                    styles,
+                    Region::new(base, Axes::splat(false)),
+                )
+            })?;
 
             if self.float {
                 frame.set_parent(self.elem.location().unwrap());
             }
 
-            Ok(frame.post_processed(self.styles))
+            Ok(frame)
         })
     }
 
diff --git a/crates/typst-layout/src/inline/collect.rs b/crates/typst-layout/src/inline/collect.rs
index fcf7508e..6023f5c6 100644
--- a/crates/typst-layout/src/inline/collect.rs
+++ b/crates/typst-layout/src/inline/collect.rs
@@ -13,6 +13,7 @@ use typst_syntax::Span;
 use typst_utils::Numeric;
 
 use super::*;
+use crate::modifiers::{layout_and_modify, FrameModifiers, FrameModify};
 
 // The characters by which spacing, inline content and pins are replaced in the
 // paragraph's full text.
@@ -36,7 +37,7 @@ pub enum Item<'a> {
     /// Fractional spacing between other items.
     Fractional(Fr, Option<(&'a Packed<BoxElem>, Locator<'a>, StyleChain<'a>)>),
     /// Layouted inline-level content.
-    Frame(Frame, StyleChain<'a>),
+    Frame(Frame),
     /// A tag.
     Tag(&'a Tag),
     /// An item that is invisible and needs to be skipped, e.g. a Unicode
@@ -67,7 +68,7 @@ impl<'a> Item<'a> {
         match self {
             Self::Text(shaped) => shaped.text,
             Self::Absolute(_, _) | Self::Fractional(_, _) => SPACING_REPLACE,
-            Self::Frame(_, _) => OBJ_REPLACE,
+            Self::Frame(_) => OBJ_REPLACE,
             Self::Tag(_) => "",
             Self::Skip(s) => s,
         }
@@ -83,7 +84,7 @@ impl<'a> Item<'a> {
         match self {
             Self::Text(shaped) => shaped.width,
             Self::Absolute(v, _) => *v,
-            Self::Frame(frame, _) => frame.width(),
+            Self::Frame(frame) => frame.width(),
             Self::Fractional(_, _) | Self::Tag(_) => Abs::zero(),
             Self::Skip(_) => Abs::zero(),
         }
@@ -210,8 +211,10 @@ pub fn collect<'a>(
                     InlineItem::Space(space, weak) => {
                         collector.push_item(Item::Absolute(space, weak));
                     }
-                    InlineItem::Frame(frame) => {
-                        collector.push_item(Item::Frame(frame, styles));
+                    InlineItem::Frame(mut frame) => {
+                        frame.modify(&FrameModifiers::get_in(styles));
+                        apply_baseline_shift(&mut frame, styles);
+                        collector.push_item(Item::Frame(frame));
                     }
                 }
             }
@@ -222,8 +225,11 @@ pub fn collect<'a>(
             if let Sizing::Fr(v) = elem.width(styles) {
                 collector.push_item(Item::Fractional(v, Some((elem, loc, styles))));
             } else {
-                let frame = layout_box(elem, engine, loc, styles, region)?;
-                collector.push_item(Item::Frame(frame, styles));
+                let mut frame = layout_and_modify(styles, |styles| {
+                    layout_box(elem, engine, loc, styles, region)
+                })?;
+                apply_baseline_shift(&mut frame, styles);
+                collector.push_item(Item::Frame(frame));
             }
         } else if let Some(elem) = child.to_packed::<TagElem>() {
             collector.push_item(Item::Tag(&elem.tag));
diff --git a/crates/typst-layout/src/inline/line.rs b/crates/typst-layout/src/inline/line.rs
index ef7e26c3..fba4bef8 100644
--- a/crates/typst-layout/src/inline/line.rs
+++ b/crates/typst-layout/src/inline/line.rs
@@ -10,6 +10,7 @@ use typst_library::text::{Lang, TextElem};
 use typst_utils::Numeric;
 
 use super::*;
+use crate::modifiers::layout_and_modify;
 
 const SHY: char = '\u{ad}';
 const HYPHEN: char = '-';
@@ -93,7 +94,7 @@ impl Line<'_> {
     pub fn has_negative_width_items(&self) -> bool {
         self.items.iter().any(|item| match item {
             Item::Absolute(amount, _) => *amount < Abs::zero(),
-            Item::Frame(frame, _) => frame.width() < Abs::zero(),
+            Item::Frame(frame) => frame.width() < Abs::zero(),
             _ => false,
         })
     }
@@ -409,6 +410,11 @@ fn should_repeat_hyphen(pred_line: &Line, text: &str) -> bool {
     }
 }
 
+/// Apply the current baseline shift to a frame.
+pub fn apply_baseline_shift(frame: &mut Frame, styles: StyleChain) {
+    frame.translate(Point::with_y(TextElem::baseline_in(styles)));
+}
+
 /// Commit to a line and build its frame.
 #[allow(clippy::too_many_arguments)]
 pub fn commit(
@@ -509,10 +515,11 @@ pub fn commit(
                 let amount = v.share(fr, remaining);
                 if let Some((elem, loc, styles)) = elem {
                     let region = Size::new(amount, full);
-                    let mut frame =
-                        layout_box(elem, engine, loc.relayout(), *styles, region)?;
-                    frame.translate(Point::with_y(TextElem::baseline_in(*styles)));
-                    push(&mut offset, frame.post_processed(*styles));
+                    let mut frame = layout_and_modify(*styles, |styles| {
+                        layout_box(elem, engine, loc.relayout(), styles, region)
+                    })?;
+                    apply_baseline_shift(&mut frame, *styles);
+                    push(&mut offset, frame);
                 } else {
                     offset += amount;
                 }
@@ -524,12 +531,10 @@ pub fn commit(
                     justification_ratio,
                     extra_justification,
                 );
-                push(&mut offset, frame.post_processed(shaped.styles));
+                push(&mut offset, frame);
             }
-            Item::Frame(frame, styles) => {
-                let mut frame = frame.clone();
-                frame.translate(Point::with_y(TextElem::baseline_in(*styles)));
-                push(&mut offset, frame.post_processed(*styles));
+            Item::Frame(frame) => {
+                push(&mut offset, frame.clone());
             }
             Item::Tag(tag) => {
                 let mut frame = Frame::soft(Size::zero());
diff --git a/crates/typst-layout/src/inline/mod.rs b/crates/typst-layout/src/inline/mod.rs
index 658e3084..bedc54d6 100644
--- a/crates/typst-layout/src/inline/mod.rs
+++ b/crates/typst-layout/src/inline/mod.rs
@@ -23,7 +23,7 @@ use typst_library::World;
 use self::collect::{collect, Item, Segment, SpanMapper};
 use self::deco::decorate;
 use self::finalize::finalize;
-use self::line::{commit, line, Line};
+use self::line::{apply_baseline_shift, commit, line, Line};
 use self::linebreak::{linebreak, Breakpoint};
 use self::prepare::{prepare, Preparation};
 use self::shaping::{
diff --git a/crates/typst-layout/src/inline/shaping.rs b/crates/typst-layout/src/inline/shaping.rs
index d6b7632b..2ed95f14 100644
--- a/crates/typst-layout/src/inline/shaping.rs
+++ b/crates/typst-layout/src/inline/shaping.rs
@@ -20,6 +20,7 @@ use unicode_bidi::{BidiInfo, Level as BidiLevel};
 use unicode_script::{Script, UnicodeScript};
 
 use super::{decorate, Item, Range, SpanMapper};
+use crate::modifiers::{FrameModifiers, FrameModify};
 
 /// The result of shaping text.
 ///
@@ -326,6 +327,7 @@ impl<'a> ShapedText<'a> {
             offset += width;
         }
 
+        frame.modify(&FrameModifiers::get_in(self.styles));
         frame
     }
 
diff --git a/crates/typst-layout/src/lib.rs b/crates/typst-layout/src/lib.rs
index 2e8c1129..56d7afe1 100644
--- a/crates/typst-layout/src/lib.rs
+++ b/crates/typst-layout/src/lib.rs
@@ -6,6 +6,7 @@ mod image;
 mod inline;
 mod lists;
 mod math;
+mod modifiers;
 mod pad;
 mod pages;
 mod repeat;
diff --git a/crates/typst-layout/src/math/fragment.rs b/crates/typst-layout/src/math/fragment.rs
index a0453c14..81b726ba 100644
--- a/crates/typst-layout/src/math/fragment.rs
+++ b/crates/typst-layout/src/math/fragment.rs
@@ -1,23 +1,22 @@
 use std::fmt::{self, Debug, Formatter};
 
 use rustybuzz::Feature;
-use smallvec::SmallVec;
 use ttf_parser::gsub::{AlternateSubstitution, SingleSubstitution, SubstitutionSubtable};
 use ttf_parser::opentype_layout::LayoutTable;
 use ttf_parser::{GlyphId, Rect};
 use typst_library::foundations::StyleChain;
 use typst_library::introspection::Tag;
 use typst_library::layout::{
-    Abs, Axis, Corner, Em, Frame, FrameItem, HideElem, Point, Size, VAlignment,
+    Abs, Axis, Corner, Em, Frame, FrameItem, Point, Size, VAlignment,
 };
 use typst_library::math::{EquationElem, MathSize};
-use typst_library::model::{Destination, LinkElem};
 use typst_library::text::{Font, Glyph, Lang, Region, TextElem, TextItem};
 use typst_library::visualize::Paint;
 use typst_syntax::Span;
 use unicode_math_class::MathClass;
 
 use super::{stretch_glyph, MathContext, Scaled};
+use crate::modifiers::{FrameModifiers, FrameModify};
 
 #[derive(Debug, Clone)]
 pub enum MathFragment {
@@ -245,8 +244,7 @@ pub struct GlyphFragment {
     pub class: MathClass,
     pub math_size: MathSize,
     pub span: Span,
-    pub dests: SmallVec<[Destination; 1]>,
-    pub hidden: bool,
+    pub modifiers: FrameModifiers,
     pub limits: Limits,
     pub extended_shape: bool,
 }
@@ -302,8 +300,7 @@ impl GlyphFragment {
             accent_attach: Abs::zero(),
             class,
             span,
-            dests: LinkElem::dests_in(styles),
-            hidden: HideElem::hidden_in(styles),
+            modifiers: FrameModifiers::get_in(styles),
             extended_shape: false,
         };
         fragment.set_id(ctx, id);
@@ -390,7 +387,7 @@ impl GlyphFragment {
         let mut frame = Frame::soft(size);
         frame.set_baseline(self.ascent);
         frame.push(Point::with_y(self.ascent + self.shift), FrameItem::Text(item));
-        frame.post_process_raw(self.dests, self.hidden);
+        frame.modify(&self.modifiers);
         frame
     }
 
@@ -516,7 +513,7 @@ impl FrameFragment {
         let base_ascent = frame.ascent();
         let accent_attach = frame.width() / 2.0;
         Self {
-            frame: frame.post_processed(styles),
+            frame: frame.modified(&FrameModifiers::get_in(styles)),
             font_size: TextElem::size_in(styles),
             class: EquationElem::class_in(styles).unwrap_or(MathClass::Normal),
             math_size: EquationElem::size_in(styles),
diff --git a/crates/typst-layout/src/math/stretch.rs b/crates/typst-layout/src/math/stretch.rs
index 6379bdb2..dafa8cbe 100644
--- a/crates/typst-layout/src/math/stretch.rs
+++ b/crates/typst-layout/src/math/stretch.rs
@@ -10,6 +10,7 @@ use super::{
     delimiter_alignment, GlyphFragment, MathContext, MathFragment, Scaled,
     VariantFragment,
 };
+use crate::modifiers::FrameModify;
 
 /// Maximum number of times extenders can be repeated.
 const MAX_REPEATS: usize = 1024;
@@ -265,7 +266,7 @@ fn assemble(
     let mut frame = Frame::soft(size);
     let mut offset = Abs::zero();
     frame.set_baseline(baseline);
-    frame.post_process_raw(base.dests, base.hidden);
+    frame.modify(&base.modifiers);
 
     for (fragment, advance) in selected {
         let pos = match axis {
diff --git a/crates/typst-layout/src/modifiers.rs b/crates/typst-layout/src/modifiers.rs
new file mode 100644
index 00000000..ac5f40b0
--- /dev/null
+++ b/crates/typst-layout/src/modifiers.rs
@@ -0,0 +1,110 @@
+use typst_library::foundations::StyleChain;
+use typst_library::layout::{Fragment, Frame, FrameItem, HideElem, Point};
+use typst_library::model::{Destination, LinkElem};
+
+/// Frame-level modifications resulting from styles that do not impose any
+/// layout structure.
+///
+/// These are always applied at the highest level of style uniformity.
+/// Consequently, they must be applied by all layouters that manually manage
+/// styles of their children (because they can produce children with varying
+/// styles). This currently includes flow, inline, and math layout.
+///
+/// Other layouters don't manually need to handle it because their parents that
+/// result from realization will take care of it and the styles can only apply
+/// to them as a whole, not part of it (since they don't manage styles).
+///
+/// Currently existing frame modifiers are:
+/// - `HideElem::hidden`
+/// - `LinkElem::dests`
+#[derive(Debug, Clone)]
+pub struct FrameModifiers {
+    /// A destination to link to.
+    dest: Option<Destination>,
+    /// Whether the contents of the frame should be hidden.
+    hidden: bool,
+}
+
+impl FrameModifiers {
+    /// Retrieve all modifications that should be applied per-frame.
+    pub fn get_in(styles: StyleChain) -> Self {
+        Self {
+            dest: LinkElem::current_in(styles),
+            hidden: HideElem::hidden_in(styles),
+        }
+    }
+}
+
+/// Applies [`FrameModifiers`].
+pub trait FrameModify {
+    /// Apply the modifiers in-place.
+    fn modify(&mut self, modifiers: &FrameModifiers);
+
+    /// Apply the modifiers, and return the modified result.
+    fn modified(mut self, modifiers: &FrameModifiers) -> Self
+    where
+        Self: Sized,
+    {
+        self.modify(modifiers);
+        self
+    }
+}
+
+impl FrameModify for Frame {
+    fn modify(&mut self, modifiers: &FrameModifiers) {
+        if let Some(dest) = &modifiers.dest {
+            let size = self.size();
+            self.push(Point::zero(), FrameItem::Link(dest.clone(), size));
+        }
+
+        if modifiers.hidden {
+            self.hide();
+        }
+    }
+}
+
+impl FrameModify for Fragment {
+    fn modify(&mut self, modifiers: &FrameModifiers) {
+        for frame in self.iter_mut() {
+            frame.modify(modifiers);
+        }
+    }
+}
+
+impl<T, E> FrameModify for Result<T, E>
+where
+    T: FrameModify,
+{
+    fn modify(&mut self, props: &FrameModifiers) {
+        if let Ok(inner) = self {
+            inner.modify(props);
+        }
+    }
+}
+
+/// Performs layout and modification in one step.
+///
+/// This just runs `layout(styles).modified(&FrameModifiers::get_in(styles))`,
+/// but with the additional step that redundant modifiers (which are already
+/// applied here) are removed from the `styles` passed to `layout`. This is used
+/// for the layout of containers like `block`.
+pub fn layout_and_modify<F, R>(styles: StyleChain, layout: F) -> R
+where
+    F: FnOnce(StyleChain) -> R,
+    R: FrameModify,
+{
+    let modifiers = FrameModifiers::get_in(styles);
+
+    // Disable the current link internally since it's already applied at this
+    // level of layout. This means we don't generate redundant nested links,
+    // which may bloat the output considerably.
+    let reset;
+    let outer = styles;
+    let mut styles = styles;
+    if modifiers.dest.is_some() {
+        reset = LinkElem::set_current(None).wrap();
+        styles = outer.chain(&reset);
+    }
+
+    layout(styles).modified(&modifiers)
+}
diff --git a/crates/typst-library/src/foundations/content.rs b/crates/typst-library/src/foundations/content.rs
index ab2f68ac..76cd6a22 100644
--- a/crates/typst-library/src/foundations/content.rs
+++ b/crates/typst-library/src/foundations/content.rs
@@ -9,7 +9,6 @@ use std::sync::Arc;
 use comemo::Tracked;
 use ecow::{eco_format, EcoString};
 use serde::{Serialize, Serializer};
-use smallvec::smallvec;
 use typst_syntax::Span;
 use typst_utils::{fat, singleton, LazyHash, SmallBitSet};
 
@@ -500,7 +499,7 @@ impl Content {
 
     /// Link the content somewhere.
     pub fn linked(self, dest: Destination) -> Self {
-        self.styled(LinkElem::set_dests(smallvec![dest]))
+        self.styled(LinkElem::set_current(Some(dest)))
     }
 
     /// Set alignments for this content.
diff --git a/crates/typst-library/src/layout/frame.rs b/crates/typst-library/src/layout/frame.rs
index e57eb27e..a26a7d0e 100644
--- a/crates/typst-library/src/layout/frame.rs
+++ b/crates/typst-library/src/layout/frame.rs
@@ -4,16 +4,13 @@ use std::fmt::{self, Debug, Formatter};
 use std::num::NonZeroUsize;
 use std::sync::Arc;
 
-use smallvec::SmallVec;
 use typst_syntax::Span;
 use typst_utils::{LazyHash, Numeric};
 
-use crate::foundations::{cast, dict, Dict, Label, StyleChain, Value};
+use crate::foundations::{cast, dict, Dict, Label, Value};
 use crate::introspection::{Location, Tag};
-use crate::layout::{
-    Abs, Axes, FixedAlignment, HideElem, Length, Point, Size, Transform,
-};
-use crate::model::{Destination, LinkElem};
+use crate::layout::{Abs, Axes, FixedAlignment, Length, Point, Size, Transform};
+use crate::model::Destination;
 use crate::text::TextItem;
 use crate::visualize::{Color, Curve, FixedStroke, Geometry, Image, Paint, Shape};
 
@@ -304,49 +301,6 @@ impl Frame {
         }
     }
 
-    /// Apply late-stage properties from the style chain to this frame. This
-    /// includes:
-    /// - `HideElem::hidden`
-    /// - `LinkElem::dests`
-    ///
-    /// This must be called on all frames produced by elements
-    /// that manually handle styles (because their children can have varying
-    /// styles). This currently includes flow, par, and equation.
-    ///
-    /// Other elements don't manually need to handle it because their parents
-    /// that result from realization will take care of it and the styles can
-    /// only apply to them as a whole, not part of it (because they don't manage
-    /// styles).
-    pub fn post_processed(mut self, styles: StyleChain) -> Self {
-        self.post_process(styles);
-        self
-    }
-
-    /// Post process in place.
-    pub fn post_process(&mut self, styles: StyleChain) {
-        if !self.is_empty() {
-            self.post_process_raw(
-                LinkElem::dests_in(styles),
-                HideElem::hidden_in(styles),
-            );
-        }
-    }
-
-    /// Apply raw late-stage properties from the raw data.
-    pub fn post_process_raw(&mut self, dests: SmallVec<[Destination; 1]>, hide: bool) {
-        if !self.is_empty() {
-            let size = self.size;
-            self.push_multiple(
-                dests
-                    .into_iter()
-                    .map(|dest| (Point::zero(), FrameItem::Link(dest, size))),
-            );
-            if hide {
-                self.hide();
-            }
-        }
-    }
-
     /// Hide all content in the frame, but keep metadata.
     pub fn hide(&mut self) {
         Arc::make_mut(&mut self.items).retain_mut(|(_, item)| match item {
diff --git a/crates/typst-library/src/model/link.rs b/crates/typst-library/src/model/link.rs
index 5df6bead..24b746b7 100644
--- a/crates/typst-library/src/model/link.rs
+++ b/crates/typst-library/src/model/link.rs
@@ -1,7 +1,6 @@
 use std::ops::Deref;
 
 use ecow::{eco_format, EcoString};
-use smallvec::SmallVec;
 
 use crate::diag::{bail, warning, At, SourceResult, StrResult};
 use crate::engine::Engine;
@@ -90,10 +89,10 @@ pub struct LinkElem {
     })]
     pub body: Content,
 
-    /// This style is set on the content contained in the `link` element.
+    /// A destination style that should be applied to elements.
     #[internal]
     #[ghost]
-    pub dests: SmallVec<[Destination; 1]>,
+    pub current: Option<Destination>,
 }
 
 impl LinkElem {
diff --git a/tests/ref/issue-758-link-repeat.png b/tests/ref/issue-758-link-repeat.png
new file mode 100644
index 00000000..aaec20d2
--- /dev/null
+++ b/tests/ref/issue-758-link-repeat.png
diff --git a/tests/ref/link-empty-block.png b/tests/ref/link-empty-block.png
new file mode 100644
index 00000000..ae10bdcf
--- /dev/null
+++ b/tests/ref/link-empty-block.png
diff --git a/tests/ref/link-on-block.png b/tests/ref/link-on-block.png
index 8fb7f6c6..eeeb264b 100644
--- a/tests/ref/link-on-block.png
+++ b/tests/ref/link-on-block.png
diff --git a/tests/suite/model/link.typ b/tests/suite/model/link.typ
index 7cced856..bd6c8a30 100644
--- a/tests/suite/model/link.typ
+++ b/tests/suite/model/link.typ
@@ -75,3 +75,14 @@ Text <hey>
 Text <hey>
 // Error: 2-20 label `<hey>` occurs multiple times in the document
 #link(<hey>)[Nope.]
+
+--- link-empty-block ---
+#link("", block(height: 10pt, width: 100%))
+
+--- issue-758-link-repeat ---
+#let url = "https://typst.org/"
+#let body = [Hello #box(width: 1fr, repeat[.])]
+
+Inline: #link(url, body)
+
+#link(url, block(inset: 4pt, [Block: ] + body))