hyperref icon indicating copy to clipboard operation
hyperref copied to clipboard

theorem's target anchor is sometimes placed in the heading of the previous page if heading contains a \vbox

Open boritomi opened this issue 1 year ago • 6 comments

Test case (pdflatex):

\documentclass{article}
\usepackage{fancyhdr}
\pagestyle{fancy}
\usepackage{hyperref}
\newtheorem{thm}{Theorem}
\textheight60pt
\begin{document}
1\\2\\3\\4\\5\\6

\begin{thm}\label{th}
Theorem text.
\end{thm}
Ref to Theorem \ref{th}.
\end{document}

Checked with recent versions, each fails since 7.00w, 7.00v (and previous versions) are OK. (Perhaps since Feb 13, 2023 [new thm implementation]?)

boritomi avatar Mar 15 '24 21:03 boritomi

I hate theorem anchors ;-). Every time you fix one instance another one breaks.

I don't think that one can really resolve all the problems through patches in hyperref. I will try to trigger a change in the LaTeX commands instead, so that the anchor is directly where it belongs and hyperref can stop to mess around.

\documentclass{article}
\makeatletter\let\@kernel@refstepcounter\refstepcounter\makeatother
\usepackage{fancyhdr}
\pagestyle{fancy}
\usepackage{hyperref}
\fancyhead{}
\makeatletter
\def\@thm#1#2{%
  \@kernel@refstepcounter{#1}%
  \@ifnextchar[{\@ythm{#1}{#2}}{\@xthm{#1}{#2}}}
\def\@begintheorem#1#2{\trivlist
   \item[\MakeLinkTarget{\@currentcounter}\hskip \labelsep{\bfseries #1\ #2}]\itshape}
\def\@opargbegintheorem#1#2#3{\trivlist
   \item[\MakeLinkTarget{\@currentcounter}\hskip \labelsep{\bfseries #1\ #2\ (#3)}]\itshape} 
          
\newtheorem{thm}{Theorem}
\textheight60pt
\begin{document}
1\\2\\3\\4\\5\\6

\begin{thm}\label{th}
Theorem text.
\end{thm}
Ref to Theorem \ref{th}.

\end{document}

u-fischer avatar Mar 18 '24 15:03 u-fischer

I hate theorem anchors ;-). Every time you fix one instance another one breaks.

I can deeply understand that!

I think that in this case there is an uglier, but in most times working workaround for this, which also deals with cases when amsthm is loaded (as was in our case, I just inserted the fancyhdr and tested it with and without amsthm loaded – in both case hyperref fails – to simplify the MWE), i.e., inserting \phantomsection automatically for the theorems:

\usepackage{fancyhdr}
\pagestyle{fancy}
%\usepackage{amsthm}
\usepackage{hyperref}
\makeatletter
\@ifpackageloaded{amsthm}{\def\@begintheorem#1#2[#3]{%
  \deferred@thm@head{\the\thm@headfont \thm@indent
    \@ifempty{#1}{\let\thmname\@gobble}{\let\thmname\@iden}%
    \@ifempty{#2}{\let\thmnumber\@gobble}{\let\thmnumber\@iden}%
    \@ifempty{#3}{\let\thmnote\@gobble}{\let\thmnote\@iden}%
    \thm@swap\swappedhead\thmhead{#1\phantomsection}{#2}{#3}%
    \the\thm@headpunct
    \thmheadnl % possibly a newline.
    \hskip\thm@headsep
  }%
  \ignorespaces}}{\def\@begintheorem#1#2{\trivlist
   \item[\hskip\labelsep{\bfseries #1\ #2}\phantomsection]\itshape}
\def\@opargbegintheorem#1#2#3{\trivlist
   \item[\hskip\labelsep{\bfseries #1\ #2\phantomsection\ (#3)}]\itshape}}
\makeatother
\newtheorem{thm}{Theorem}
\textheight60pt
\begin{document}
1\\2\\3\\4\\5\\6

\begin{thm}\label{th}
Theorem text.
\end{thm}
Ref to Theorem \ref{th}.
\end{document}

But it is just a temporary workaround. In the long run, that would be very nice if hyperref could deal with theorems' target anchors better...

boritomi avatar Mar 19 '24 13:03 boritomi

inserting \phantomsection automatically for the theorems:

That is basically what my code above is doing: \item[\MakeLinkTarget{\@currentcounter}\hskip \labelsep{\bfseries #1\ #2\ (#3)}]\itshape} (and imho the target should be at the begin of the label and not at the end.)

But \MakeLinkTarget is better as it works also without hyperref.

In the long run, that would be very nice if hyperref could deal with theorems' target anchors better...

No in the long run it would be better if hyperref wouldn't have to deal with that at all. It is simply a pain to have to patch these commands from the outside. It would be much better if amsthm etc would insert the targets directly.

I have now opened a pull request to change at least the kernel definitions. https://github.com/latex3/latex2e/pull/1301.

u-fischer avatar Mar 19 '24 14:03 u-fischer

No in the long run it would be better if hyperref wouldn't have to deal with that at all. It is simply a pain to have to patch these commands from the outside. It would be much better if amsthm etc would insert the targets directly.

I have now opened a pull request to change at least the kernel definitions. latex3/latex2e#1301.

I have to agree, however, it seems that your patch will break amsthm and maybe other libraries redefining \@begintheorem (or maybe also \@thm, \@opargbegintheorem, etc.) (try your code with \usepackage{amsthm} in the preamble).

boritomi avatar Mar 19 '24 16:03 boritomi

I have to agree, however, it seems that your patch will break amsthm

No, the kernel change should not affect them as they overwrite the definitions later (I checked that ...). So they only loose the target again.

u-fischer avatar Mar 19 '24 16:03 u-fischer

with a current latex-dev (released a few day ago) this now works correctly.

u-fischer avatar Mar 29 '24 22:03 u-fischer