Premature expansion in June 2022 MakeUpper/Lowercase

[davidcarlisle]

Brief outline of the bug

Originally reported at https://tex.stackexchange.com/a/649023

The new \MakeUpperCase is documented as obsoleting the textcase package but while the underlying l3 case change code does have support for protecting the arguments of \label and \ref etc from case changes, the outer \protected@edef added for legacy LICR handling pre-expands these commands and things go badly wrong.

The example below shows the error and if the first line is changed to \iftrue a suggested fix is enabled.

The fix re-instates the protection of \cite from textcase but re-implemented to use \unexpanded and as a loop over the same list of commands made safe in the l3 code.

A 2e command \AddToNoCaseChangeList is also provided to make it easy to add to that list without explicit l3 code syntax.

An example would be \AddToNoCaseChangeList{\eqref} to add \eqref to the list. amsmath could be updated to execute this (if it is defined) to add its cross reference commands automatically

Minimal example showing the bug

\iffalse


\makeatletter
\DeclareRobustCommand{\MakeUppercase}[1]{{%
      \def\i{I}\def\j{J}%
      \def\reserved@a##1##2{\let##1##2\reserved@a}%
      \expandafter\reserved@a\@uclclist\reserved@b{\reserved@b\@gobble}%
      \begingroup
        \@process@text@case@exclude@arg@tl
      \protected@edef\reserved@a{\endgroup
   \@expl@text@uppercase@@n{\noexpand\unexpanded{#1}}}%
    \reserved@a
   }}

\def\unexpand@arg@loop#1{\ifx#1\@nnil
  \else\def#1{\unexpand@arg#1}\expandafter\unexpand@arg@loop
  \fi}
\def\unexpand@arg#1#2#{\unexpand@arg@b#1{#2}}
\def\unexpand@arg@b#1#2#3{\unexpanded{#1#2{#3}}}


\ExplSyntaxOn
\def\AddToNoCaseChangeList{\tl_put_right:Nn \l_text_case_exclude_arg_tl}
\def\@process@text@case@exclude@arg@tl{
   \expandafter\unexpand@arg@loop\l_text_case_exclude_arg_tl\@nnil}
\ExplSyntaxOff

\makeatother


\fi

\RequirePackage{latexbug}
\documentclass{article}

\protected\def\eqref#1{(\ref{#1})}

\ifx\AddToNoCaseChangeList\undefined\else
\AddToNoCaseChangeList{\eqref}
\fi

\begin{document}


\section{abc}\label{sec:a}

\begin{equation}
  \label{eq:a}
  1=1
\end{equation}
\MakeUppercase{xyz \ref{sec:a} and \eqref{eq:a} \label{lower}}

see \ref{lower}

\end{document}

Log file (required) and possibly PDF file

log errors muliple spurious errors starting

LaTeX Warning: Reference `EQ:A' on page 1 undefined on input line 54.

! Missing control sequence inserted.
<inserted text> 
                \inaccessible 
l.54 ...ref{sec:a} and \eqref{eq:a} \label{lower}}
                                                  
? 
! Use of \let doesn't match its definition.
<inserted text> ...LABEL{LOWER}{{1}{1}}}\let \let 
                                                  \endgroup \let \let \relax...
l.54 ...ref{sec:a} and \eqref{eq:a} \label{lower}}
                                                  
? 

[josephwright]

Remind me why we need the \protected@edef ...

[davidcarlisle]

Remind me why we need the \protected@edef

Mostly I think to handle legacy encoded input expanding to LICR which gets changed via the origial uclc list.

[josephwright]

Do we have a test for that? I'd like to see if I can just fix in the expl3 code at low cost

[josephwright]

Ah, found one: tlb2529.

[josephwright]

That seems to be the only one ... and I'm not really sure I get what it's up to. Doesn't exactly seem to test what I am trying to verify!

[josephwright]

Since the expl3 code already handles \label, etc., we could look to remove the \protetected@edef. That works fine for UTF-8, while to extend for other input encodings I think by allowing expansion of active chars, something like

\cs_gset:Npn \__text_expand_explicit:N #1
  {
    \token_if_cs:NTF #1
      {\__text_expand_exclude:N #1}
      {
        \bool_lazy_and:nnTF
          { \token_if_active_p:N #1 }
          {
            ! \bool_lazy_or_p:nn
              { \token_if_protected_macro_p:N #1 }
              { \token_if_protected_long_macro_p:N #1 }
          }
          { \exp_after:wN \__text_expand_loop:w #1 }
          { \__text_expand_store:n {#1} \__text_expand_loop:w }
          
      }
  }

How does that look?

[davidcarlisle]

@josephwright if we go that way, MakeUppercase simplifies a lot but presumably a bit slower with legacy input encodings. From this PR we could still keep \AddToNoCaseChangeList to provide a 2e interface for adding to the list, is that a good name?

[josephwright]

@davidcarlisle I think the name is fine - certainly I don't have a better one and it's clear what it does. On the change of speed, supporting in the expl3 code is slower than \protected@edef but the current approach still runs the expander inside the expl3 code. So the cost is likely marginal. (It would be different if we had a 'no expansion' version of \text_<thing>case:n, but that seems like a risky plan to me - two almost-identical code paths but with crucial differences.)

[josephwright]

I can change the expl3 today - that can be done independent of the 2e change. But I really could do with more than the one test case I quickly created.

[davidcarlisle]

I think @josephwright 's suggestion would be good. This test file shows several issues with the existing edef. test3 shows \protect\foo keeps latin script lowercase but uppercases cyrillic as it is expanded while the local setting of \cyra to \CYRA is in effect. LaTeX has always done this unrelated to recent changes, but it does not seem documented or justifiable. Directly using the l3 version as in test 4 produces the (shown) expected outcome. (The file is set up to be easily converted to an l3build test but just view the pdf output to see the result.)

\documentclass{article}

\setlength\parindent{0pt}
\setlength\parskip{20pt}

\ExplSyntaxOn
\let\textuppercase\text_uppercase:n
\ExplSyntaxOff

\scrollmode
%\input{test2e}%\errorstopmode

\usepackage[T2A]{fontenc}

\def\TEST#1#2{\sbox0{#1}\showbox0 \usebox{0}\\$\rightarrow$ #2}

\begin{document}

\TEST{0: \CYRA\CYRB\CYRV}
         {\CYRA\CYRB\CYRV}

\TEST{1: \MakeUppercase{\cyra\cyrb\cyrv}}
         {\CYRA\CYRB\CYRV}

\TEST{2: \textuppercase{\cyra\cyrb\cyrv}}
         {\CYRA\CYRB\CYRV}

\newcommand\foo{abc\cyra\cyrb\cyrv}

\TEST{3: \MakeUppercase{lower: \protect\foo\ upper: \foo}}
           {LOWER: abc\cyra\cyrb\cyrv\ UPPER: ABC\CYRA\CYRB\CYRV}

\TEST{4: \textuppercase{lower: \protect\foo\ upper: \foo}}
           {LOWER: abc\cyra\cyrb\cyrv\ UPPER: ABC\CYRA\CYRB\CYRV}
	   
\end{document}

[John02139]

I've loaded PL4 and tested the code. The issues with eqref and label are fixed. I noticed an issue with footnotes in section headings if the titlesec package is loaded. Without loading titlesec, a footnote in a section heading does not require \protect; if titlesec is loaded, it does. (In both cases, I found it necessary to robustify the footnote.) The need to protect is new (was not present before June 2022).

The following illustrates.

\documentclass[11pt]{article}

% the footnote must be made robust, with or without titlesec
\usepackage{etoolbox}
\robustify{\footnote}

\AddToNoCaseChangeList{\footnote}

\usepackage[raggedright,indentafter]{titlesec}
\titleformat{\section}{\bfseries\sffamily\raggedright}{\thesection .}{0.5em}{\MakeUppercase}

\begin{document}
%\section[Section]{Heading with footnote\footnote{The first footnote.}}% <== works fine without loading titlesec
Without loading \texttt{titlesec}, a simple footnote works.
\MakeUppercase{The section begins.\footnote{The second footnote.}}

\section[Section]{Heading with protected footnote\protect\footnote{The first footnote.}}% <== \protect needed with titlesec
When \texttt{titlesec} is loaded, the footnote in the section heading must be protected.
\MakeUppercase{The section begins.\footnote{The second footnote.}}

\end{document}

[davidcarlisle]

@John02139 titlesec unprotects \footnote I'll ping @jbezos but could you test your local files after changing line 286 of a copy of titlsec.sty

from

\def\footnote{\@ifnextchar[%

to

\protected\def\footnote{\@ifnextchar[%

[John02139]

@davidcarlisle -- I've made that change in a local copy of titlesec.sty and tried it with both the small file posted here and my production files. The change fully resolves the issue.

[davidcarlisle]

@John02139 thanks for the report, I'll close here, as you may have seen I opened an issue at titlesec to track your final issue.