Forbid footnote migration in pending floats (#5497)

Co-authored-by: Laurenz <laurmaedje@gmail.com>

crates/typst-layout/src/flow/compose.rs

@@ -214,6 +214,13 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> {
     }
 
     /// Lay out the inner contents of a column.
+    ///
+    /// Pending floats and footnotes are also laid out at this step. For those,
+    /// however, we forbid footnote migration (moving the frame containing the
+    /// footnote reference if the corresponding entry doesn't fit), allowing
+    /// the footnote invariant to be broken, as it would require handling a
+    /// [`Stop::Finish`] at this point, but that is exclusively handled by the
+    /// distributor.
     fn column_contents(&mut self, regions: Regions) -> FlowResult<Frame> {
         // Process pending footnotes.
         for note in std::mem::take(&mut self.work.footnotes) {
@@ -222,7 +229,7 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> {
 
         // Process pending floats.
         for placed in std::mem::take(&mut self.work.floats) {
-            self.float(placed, &regions, false)?;
+            self.float(placed, &regions, false, false)?;
         }
 
         distribute(self, regions)
@@ -236,13 +243,21 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> {
     /// (depending on `placed.scope`).
     ///
     /// When the float does not fit, it is queued into `work.floats`. The
-    /// value of `clearance` that between the float and flow content is needed
-    /// --- it is set if there are already distributed items.
+    /// value of `clearance` indicates that between the float and flow content
+    /// is needed --- it is set if there are already distributed items.
+    ///
+    /// The value of `migratable` determines whether footnotes within the float
+    /// should be allowed to prompt its migration if they don't fit in order to
+    /// respect the footnote invariant (entries in the same page as the
+    /// references), triggering [`Stop::Finish`]. This is usually `true` within
+    /// the distributor, as it can handle that particular flow event, and
+    /// `false` elsewhere.
     pub fn float(
         &mut self,
         placed: &'b PlacedChild<'a>,
         regions: &Regions,
         clearance: bool,
+        migratable: bool,
     ) -> FlowResult<()> {
         // If the float is already processed, skip it.
         let loc = placed.location();
@@ -291,7 +306,7 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> {
         }
 
         // Handle footnotes in the float.
-        self.footnotes(regions, &frame, need, false)?;
+        self.footnotes(regions, &frame, need, false, migratable)?;
 
         // Determine the float's vertical alignment. We can unwrap the inner
         // `Option` because `Custom(None)` is checked for during collection.
@@ -326,12 +341,19 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> {
     /// Lays out footnotes in the `frame` if this is the root flow and there are
     /// any. The value of `breakable` indicates whether the element that
     /// produced the frame is breakable. If not, the frame is treated as atomic.
+    ///
+    /// The value of `migratable` indicates whether footnote migration should be
+    /// possible (at least for the first footnote found in the frame, as it is
+    /// forbidden for the second footnote onwards). It is usually `true` within
+    /// the distributor and `false` elsewhere, as the distributor can handle
+    /// [`Stop::Finish`] which is returned when migration is requested.
     pub fn footnotes(
         &mut self,
         regions: &Regions,
         frame: &Frame,
         flow_need: Abs,
         breakable: bool,
+        migratable: bool,
     ) -> FlowResult<()> {
         // Footnotes are only supported at the root level.
         if !self.config.root {
@@ -352,7 +374,7 @@ impl<'a, 'b> Composer<'a, 'b, '_, '_> {
 
         let mut relayout = false;
         let mut regions = *regions;
-        let mut migratable = !breakable && regions.may_progress();
+        let mut migratable = migratable && !breakable && regions.may_progress();
 
         for (y, elem) in notes {
             // The amount of space used by the in-flow content that contains the

crates/typst-layout/src/flow/distribute.rs

@@ -240,7 +240,8 @@ impl<'a, 'b> Distributor<'a, 'b, '_, '_, '_> {
 
         // Handle fractionally sized blocks.
         if let Some(fr) = single.fr {
-            self.composer.footnotes(&self.regions, &frame, Abs::zero(), false)?;
+            self.composer
+                .footnotes(&self.regions, &frame, Abs::zero(), false, true)?;
             self.flush_tags();
             self.items.push(Item::Fr(fr, Some(single)));
             return Ok(());
@@ -323,8 +324,13 @@ impl<'a, 'b> Distributor<'a, 'b, '_, '_, '_> {
         }
 
         // Handle footnotes.
-        self.composer
-            .footnotes(&self.regions, &frame, frame.height(), breakable)?;
+        self.composer.footnotes(
+            &self.regions,
+            &frame,
+            frame.height(),
+            breakable,
+            true,
+        )?;
 
         // Push an item for the frame.
         self.regions.size.y -= frame.height();
@@ -347,11 +353,13 @@ impl<'a, 'b> Distributor<'a, 'b, '_, '_, '_> {
                 placed,
                 &self.regions,
                 self.items.iter().any(|item| matches!(item, Item::Frame(..))),
+                true,
             )?;
             self.regions.size.y -= weak_spacing;
         } else {
             let frame = placed.layout(self.composer.engine, self.regions.base())?;
-            self.composer.footnotes(&self.regions, &frame, Abs::zero(), true)?;
+            self.composer
+                .footnotes(&self.regions, &frame, Abs::zero(), true, true)?;
             self.flush_tags();
             self.items.push(Item::Placed(frame, placed));
         }

crates/typst-layout/src/pages/collect.rs

@@ -53,7 +53,7 @@ pub fn collect<'a>(
 
             // The initial styles for the next page are ours unless this is a
             // "boundary" pagebreak. Such a pagebreak is generated at the end of
-            // the scope of a page set rule to ensure a page boundary. It's
+            // the scope of a page set rule to ensure a page boundary. Its
             // styles correspond to the styles _before_ the page set rule, so we
             // don't want to apply it to a potential empty page.
             if !pagebreak.boundary(styles) {

crates/typst-library/src/model/footnote.rs

@@ -194,7 +194,7 @@ cast! {
 /// before any page content, typically at the very start of the document.
 #[elem(name = "entry", title = "Footnote Entry", Show, ShowSet)]
 pub struct FootnoteEntry {
-    /// The footnote for this entry. It's location can be used to determine
+    /// The footnote for this entry. Its location can be used to determine
     /// the footnote counter state.
     ///
     /// ```example

tests/suite/layout/flow/footnote.typ

@@ -295,3 +295,23 @@ A #footnote(numbering: "*")[B]<fn>, C @fn, D @fn, E @fn.
 --- issue-5256-multiple-footnotes-in-footnote ---
 // Test whether all footnotes inside another footnote are listed.
 #footnote[#footnote[A]#footnote[B]#footnote[C]]
+
+--- issue-5435-footnote-migration-in-floats ---
+// Test that a footnote should not prompt migration when in a float that was
+// queued to the next page (due to the float being too large), even if the
+// footnote does not fit, breaking the footnote invariant.
+#set page(height: 50pt)
+
+#place(
+  top,
+  float: true,
+  {
+    v(100pt)
+    footnote[a]
+  }
+)
+#place(
+  top,
+  float: true,
+  footnote[b]
+)