Gh836 (#837)

* test file showing the problem * fix for #836 * __mark -> @@ and other minor fixes Co-authored-by: PhelypeOleinik <phelype.oleinik@latex-project.org>
latex3 · Jun 2, 2022 · 878a471 · 878a471
1 parent 832b960
commit 878a471
Show file tree

Hide file tree

Showing 6 changed files with 204 additions and 13 deletions.
diff --git a/base/changes.txt b/base/changes.txt
@@ -11,6 +11,12 @@ are not part of the distribution.
 	* ltfinal.dtx: add \NoCaseChange (previously available in the
           textcase package)
 
+2022-06-01  Frank Mittelbach  <Frank.Mittelbach@latex-project.org>
+
+	* ltmarks.dtx (subsection{Updating mark structures}):
+	Be more careful when unpackage a \vbox for mark detection, it might
+	contain infinite shrink glue (gh/836)
+
 2022-05-27  David Carlisle  <David.Carlisle@latex-project.org>
 
 	* ltfiles.dtx, ltoutenc.dtx:

diff --git a/base/ltmarks.dtx b/base/ltmarks.dtx
@@ -14,8 +14,8 @@
 %%% From File: ltmarks.dtx
 %
 %    \begin{macrocode}
-\def\ltmarksversion{v1.0c}
-\def\ltmarksdate{2022/05/06}
+\def\ltmarksversion{v1.0d}
+\def\ltmarksdate{2022/06/01}
 %    \end{macrocode}
 %<*driver>
 \documentclass{l3doc}
@@ -826,18 +826,25 @@
       \dim_set_eq:NN \tex_vfuzz:D         \c_max_dim
 %    \end{macrocode}
 %    There is a further complication: if the region contains infinite
-%    shrinking glue then a \cs{vsplit} operation will balk with a
+%    shrinking glue then a \tn{vsplit} operation will balk with a
 %    low-level error. Now pages or columns, which are our main concern here, can't
 %    have such infinite shrinkage if they are cut straight from the
-%    galley, however the use of \cs{enlargethispage} actually does add
+%    galley, however the use of \tn{enlargethispage} actually does add
 %    some at the very bottom (and also wraps the whole page into a box
 %    by itself, so if we leave it this way then a) we get this error
 %    and b) we don't see any marks because they are hidden one level
 %    down).
+%      
+%    Another possible issue are packages or user code that place stray
+%    \tn{vbox}es directly into the main galley (an example is
+%    \pkg{marginnote} that attaches its marginals in this way). If such
+%    boxes end up as the last item on the page we should not unpack
+%    them.
 %
 %    We therefore do an \tn{unskip} to get rid of that glue if present and
-%    also check if we have then a \cs{vbox} as the last item and if so
-%    unpack that too. All this is temporary, just for getting the
+%    also check if we have then a \tn{vbox} as the last item and if so
+%    unpack that too, but only under certain conditions, see
+%    below. All this is temporary, just for getting the
 %    marks out, so it doesn't affect the final page production.
 %
 %    In fact, we go one step further and set the box to a large
@@ -856,16 +863,50 @@
           #2
           \tex_unskip:D
           \box_set_to_last:N \l_@@_box
-          \box_if_vertical:NT \l_@@_box
-              { \vbox_unpack:N \l_@@_box }
+%    \end{macrocode}
+%    After having removed the last box from the current list (if there
+%    was one)
+%    we check if the list is now empty. If not, the the last box is
+%    definitely not the one from \tn{enlargethispage} and so we can
+%    and should leave it alone. Otherwise we check if this last box is
+%    a \tn{vbox}.
+% \changes{v1.0d}{2022/06/01}{Extend the logic for detecting the marks
+%    in the box (gh/836)}
+%    \begin{macrocode}
+          \int_compare:nNnT \tex_lastnodetype:D < 0
+            {
+              \box_if_vertical:NT \l_@@_box
+                {
+%    \end{macrocode}
+%    If it is we do a further test and reset the \cs{l_@@_box}
+%    to check if it contains infinitely shrinkable glue.
+%    \begin{macrocode}
+                  \vbox_set_to_ht:Nnn \l_@@_box { -.5\c_max_dim }
+                    {
+                      \vbox_unpack:N \l_@@_box
+                      \tex_kern:D \c_zero_dim % ensure that box
+                                              % is not empty
+                    }
+%    \end{macrocode}
+%    If not, then we unpack it, if yes we still ignore it for the process of
+%    mark extraction. We do not generate an error though, because in all
+%    likelihood this is an ordinary box like a marginal that does
+%    contain something like \tn{vss}.
+%    \begin{macrocode}
+                    \int_compare:nNnT \tex_badness:D > 0
+                      { \vbox_unpack:N \l_@@_box }
+                  }
+            }
 %    \end{macrocode}
 %    If it wasn't a vbox, it was either an hbox or there was no box.
 %    Given that we are only interested in the marks we don't need put
-%    it back in that case. However, we have to make sure that the box
+%    it back in that case. However, we have to make sure that the
+%    outer box under construction
 %    is not totally empty (which it might have been from the start, or
 %    now), because \TeX{} does not report a badness for empty boxes
-%    which means out test would incorrectly conclude that we have
-%    infinite shrinking glue. A simple \tn{kern} is enough to avoid this.
+%    and that means our test would incorrectly conclude that we have
+%    infinite shrinking glue. A simple \tn{kern} is enough to avoid
+%    this (the same was already done above).
 %    \begin{macrocode}
           \tex_kern:D \c_zero_dim
         }
@@ -1165,6 +1206,12 @@
 %
 % \subsection{Messages}
 %
+%    Mark errors are LaTeX kernel errors:
+% \changes{v1.0d}{2022/06/01}{Marks are kernel errors}
+%    \begin{macrocode}
+\prop_gput:Nnn \g_msg_module_type_prop { mark } { LaTeX }
+%    \end{macrocode}
+%
 %    \begin{macrocode}
 \msg_new:nnnn { mark } { class-already-defined }
   { Mark~class~'#1'~already~defined }

diff --git a/base/testfiles-ltmarks/github-0836.lvt b/base/testfiles-ltmarks/github-0836.lvt
@@ -0,0 +1,41 @@
+%
+% from Ulrike
+%
+
+
+
+\documentclass{book}
+
+\newbox\mybox
+
+\makeatletter
+\AddToHook{cmd/@opcol/before}{{\showoutput\showbox\@outputbox}}
+\makeatother
+
+\input{regression-test}
+
+\begin{document}
+
+\START
+
+% mimic marginnote ...
+
+x 
+
+\vspace{44\baselineskip}
+
+\setbox\mybox\vbox to \ht\strutbox{blub\vss}%
+\box\mybox\vskip-\baselineskip
+blub \\zzz
+\newpage
+
+% trival case:
+
+\vbox to 1cm{xxx\vss}
+\break
+
+\OMIT
+\end{document}
+
+
+
diff --git a/base/testfiles-ltmarks/github-0836.tlg b/base/testfiles-ltmarks/github-0836.tlg
@@ -0,0 +1,95 @@
+This is a generated file for the l3build validation system.
+Don't change this file in any respect.
+> \box...=
+\vbox(550.0+0.0)x345.0
+.\write-{}
+.\glue(\topskip) 5.69446
+.\hbox(4.30554+0.0)x345.0, glue set 324.7222fil
+..\hbox(0.0+0.0)x15.0
+..\OT1/cmr/m/n/10 x
+..\penalty 10000
+..\glue(\parfillskip) 0.0 plus 1.0fil
+..\glue(\rightskip) 0.0
+.\glue 528.0
+.\glue 0.0
+.\glue(\baselineskip) 3.60004
+.\vbox(8.39996+0.0)x345.0, glue set 1.45552fil
+..\hbox(6.94444+0.0)x345.0, glue set 310.5555fil
+...\hbox(0.0+0.0)x15.0
+...\OT1/cmr/m/n/10 b
+...\OT1/cmr/m/n/10 l
+...\OT1/cmr/m/n/10 u
+...\OT1/cmr/m/n/10 b
+...\penalty 10000
+...\glue(\parfillskip) 0.0 plus 1.0fil
+...\glue(\rightskip) 0.0
+..\glue 0.0 plus 1.0fil minus 1.0fil
+.\glue 0.0
+! OK.
+\__hook_toplevel cmd/@opcol/before ...\@outputbox 
+                                                  }
+l. ...\newpage
+[1
+]
+> \box...=
+\vbox(550.0+0.0)x345.0, glue set 528.0fil
+.\glue(\topskip) 3.05556
+.\hbox(6.94444+0.0)x345.0, glue set 310.5555fil
+..\hbox(0.0+0.0)x15.0
+..\OT1/cmr/m/n/10 b
+..\OT1/cmr/m/n/10 l
+..\OT1/cmr/m/n/10 u
+..\OT1/cmr/m/n/10 b
+..\penalty 10000
+..\glue 0.0 plus 1.0fil
+..\penalty -10000
+..\glue(\rightskip) 0.0
+.\penalty 300
+.\glue(\baselineskip) 7.69446
+.\hbox(4.30554+0.0)x345.0, glue set 331.66667fil
+..\OT1/cmr/m/n/10 z
+..\OT1/cmr/m/n/10 z
+..\OT1/cmr/m/n/10 z
+..\penalty 10000
+..\glue(\parfillskip) 0.0 plus 1.0fil
+..\glue(\rightskip) 0.0
+.\glue 0.0 plus 1.0fil
+.\glue 0.0
+! OK.
+\__hook_toplevel cmd/@opcol/before ...\@outputbox 
+                                                  }
+l. ...\newpage
+[2]
+Underfull \vbox (badness 10000) has occurred while \output is active
+\vbox(550.0+0.0)x345.0
+.\glue(\topskip) 0.0
+.\vbox(28.45274+0.0)x345.0, glue set 24.1472fil
+..\hbox(4.30554+0.0)x345.0, glue set 314.1666fil
+...\hbox(0.0+0.0)x15.0
+...\OT1/cmr/m/n/10 x
+...\OT1/cmr/m/n/10 x
+...\OT1/cmr/m/n/10 x
+...\penalty 10000
+...\glue(\parfillskip) 0.0 plus 1.0fil
+...\glue(\rightskip) 0.0
+..\glue 0.0 plus 1.0fil minus 1.0fil
+.\glue 0.0
+> \box...=
+\vbox(550.0+0.0)x345.0
+.\glue(\topskip) 0.0
+.\vbox(28.45274+0.0)x345.0, glue set 24.1472fil
+..\hbox(4.30554+0.0)x345.0, glue set 314.1666fil
+...\hbox(0.0+0.0)x15.0
+...\OT1/cmr/m/n/10 x
+...\OT1/cmr/m/n/10 x
+...\OT1/cmr/m/n/10 x
+...\penalty 10000
+...\glue(\parfillskip) 0.0 plus 1.0fil
+...\glue(\rightskip) 0.0
+..\glue 0.0 plus 1.0fil minus 1.0fil
+.\glue 0.0
+! OK.
+\__hook_toplevel cmd/@opcol/before ...\@outputbox 
+                                                  }
+l. ...\break
+[3]
diff --git a/base/testfiles-ltmarks/xmarks-001.tlg b/base/testfiles-ltmarks/xmarks-001.tlg
@@ -146,7 +146,7 @@ Marks: baz in OR (oneside):
     column (first):||first baz|first baz|
     column (second):||first baz|first baz|
 [3]
-! Package mark Error: Unknown mark class 'unknown'.
+! LaTeX mark Error: Unknown mark class 'unknown'.
 For immediate help type H <return>.
  ...                                              
 l. ...

diff --git a/base/update-ltmarks-test.sh b/base/update-ltmarks-test.sh
@@ -7,7 +7,9 @@ l3build save -cconfig-ltmarks \
 	xmarks-005 \
 	xmarks-006 \
 	xmarks-007 \
-	xmarks-008
+	xmarks-008 \
+	github-0836
+
 
 exit