The Script option breaks after v2.6j update: Debug overkill?

RuixiZhang42 commented 5 years ago

Description

fontspec v2.6j cannot silently ignore non-existing scripts.

Check/indicate

[x] Relevant for XeTeX
[x] Issue tracker has been searched for similar issues?

Minimal example demonstrating the issue

% !TeX program = XeLaTeX
\documentclass{article}
\usepackage{fontspec}% 2019/01/10 v2.6j
\setmainfont{FandolSong}[Script=CJK]
\begin{document}
Hello, world!
\end{document}

Further details

The above MWE complies without any errors under LuaLaTeX with fontspec v2.6j. Under XeLaTeX, I got the following message:

Package fontspec Warning: Font 'FandolSong' does not contain script 'CJK'.

)
Runaway argument?
\int_set:Nn \l_tmpb_int {\XeTeXOTcountfeatures \l_fontspec_font \l__fontspec_sc
ript_int \ETC.
! File ended while scanning use of \__fontspec_strip_leading_sign:Nw.
<inserted text> 
                \par

There were no errors with fontspec v2.6i under both engines. This update affects xeCJK which passes Script=CJK by default, e.g., this issue.

stone-zeng commented 5 years ago

The problem is in \@@_iv_str_to_num:Nn:

https://github.com/wspr/fontspec/blob/66c7a9eb7d7bde5383e046feb505c0121a53fc36/fontspec-code-internal.dtx#L1109-L1112

When #2 is empty (e.g. for Fandol font), it will break. Function \@@_strip_leading_sign:Nw and \@@_iv_str_to_num:w have the similar risk to break when eating empty arguments.

The bug is introduced in fcc5a51feb7016f1a1b961c56830fa2ccc4b8db2.

stone-zeng commented 5 years ago

The issue is still here with fontspec v2.6k...

Try the example above, I get an infinite loop of warning:

Package fontspec Warning: Font 'FandolSong' does not contain script 'CJK'.
Package fontspec Warning: Font 'FandolSong' does not contain script 'Default'.
Package fontspec Warning: Font 'FandolSong' does not contain script 'Default'.
...

wspr commented 5 years ago

@Stone-Zeng Oh dear, sorry. It's too late for me to fix now but it's pretty clear what went wrong. I thought I had a test file to catch this and made a mistake :( Will hopefully be able to fix tomorrow...

RuixiZhang42 commented 5 years ago

Oh dear! This breaks again in v2.7 under XeTeX (and LuaTeX is fine). I got the following error message:

Package fontspec Warning: Font "FandolSong" does not contain requested Script
(fontspec)                "".

! LaTeX Error: Missing \begin{document}.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H <return>  for immediate help.
 ...                                              

l.4 \setmainfont{FandolSong}[Script=CJK]

If I ignore the message and proceed to compile, I get the following surprising output suggesting that the string “CJK” is somehow leaking:

qqq

wspr commented 5 years ago

Just the warning is broken, right? My test file seems to compile okay?

wspr commented 5 years ago

Oh, damn. No, it’s still broken. How could this happen? :)

My test file loaded the font after \begin{document}. Er… will fix ASAP.

RuixiZhang42 commented 5 years ago

My observation was that the string CJK was missing in the log as Font "FandolSong" does not contain requested Script "", but somehow made its way to the PDF, i.e., the CJK in front of the Hello. Hope this will provide useful information about the bug.

RuixiZhang42 commented 5 years ago

The good news is that the other (later) script/language related issues stay fixed :)

wspr commented 5 years ago

Okay, once again this is now fixed :) I’ll release later today assuming nothing else comes up. The CTAN folk must be getting pretty annoyed with me.

RuixiZhang42 commented 5 years ago

I think I have one last thing (more of a question actually).

With the working code, under XeTeX FandolSong is loaded without any script or language:

(fontspec)             - 'normal' (m/n) with NFSS spec.:
(fontspec)             <->"FandolSong/OT:mapping=tex-text;"

while under LuaTeX the script and the language are present:

(fontspec)             - 'normal' (m/n) with NFSS spec.:
(fontspec)             <->"FandolSong:mode=node;script=hani;language=DFLT;+tlig;"

Compilation went through without any errors under both engines. Is there no need to pass language=DFLT to FandolSong under XeTeX? (I’m not sure…)

wspr commented 5 years ago

Good question. Actually I have a feeling that is revealing a bug in the luatex code.

The language tag is explicitly tied to the script tag, so when the hani script is not found the language tag is dropped too.

In luatex I would expect the same to happen. Let me see...

wspr commented 5 years ago

If anything this is a bug in luaotfload. I’m afraid I don’t have time to investigate further — my last push has a new test script which shows that, for some reason, luaotfload is saying that FandolSong DOES have latn and hani scripts but not cyrl.

This could be expected behaviour (perhaps it’s constructing the scripts dynamically based on the glyphs in the font).

Regardless, there’s no problem AFAIK to specify an unknown/missing script or language — all that will happen is the engine will fall back to the default option.

RuixiZhang42 commented 5 years ago

Yes, the new test files showed the discrepancies between the two engines when facing FandolSong. I agree that fontspec behaves as expected, phew…

latex3 / fontspec