Should error for <span> that fails to xref

[zcorpan]

<span>no such term</span> should be an error.

[annevk]

So the solution is in changing line 454. However, I cannot get

if (Element.HasAttribute(kCrossRefAttribute) or Element.IsIdentity(nsHTML, eCode) or (Element.IsIdentity(nsHTML, eSpan) and Element.Attributes.Count = 0)) then

or variants to compile. I'm not sure what's wrong since Attributes.Count is compared as a number elsewhere just fine.

@Hixie can you help?

(We need to ignore <span> elements with attributes, due to classes and lang attributes and such. And I don't think we want to exclude those attributes explicitly.)

[sideshowbarker]

I remember running into a similar problem before.

To fix it I think you need to do this (wrapped for readability):

if (Element.HasAttribute(kCrossRefAttribute) or Element.IsIdentity(nsHTML, eCode)
    or (Element.IsIdentity(nsHTML, eSpan) and (Element.Attributes.Count = 0))) then

That is, with parens in "and (Element.Attributes.Count = 0)".

Otherwise without the parens, if there’s a bare identifier after the and, I think in Pascal the order of evaluation is, the compiler tries to evaluate that identifier on its own as a boolean. And it finds Element.Attributes.Count is not a boolean, so it bails.

So I think in Pascal to make a compiler evaluate the Element.Attributes.Count = 0 expression first, as whole—and not prematurely evaluate Element.Attributes.Count on its own—parens are needed.

[annevk]

I tried that, same or nearly the same result unfortunately.

[annevk]

To be clear, that makes it compile, but running it results in:

EAccessViolation exception: Access violation
Backtrace:
  $00019C49
  $000172BC
  $0001408E
  $0001182D
  $00025F0B

[annevk]

The overall problem was that an Element without attributes never gets Attributes assigned. So the correct way would be (not Assigned(Element.Attributes)).

[Hixie]

Isn't there a HasAttributes?

[annevk]

I don't think so. At least your code also does this Assigned trick in a number of places.

[sideshowbarker]

(meta point) I guess we could add a HasAttributes that returns boolean, but even if we did I wonder what other need we’d ever end up having for it.

The need right now is that we want to:

A. identify and report cases of xref spans like "<span>Clear regions that cover the pixels</span>" that don’t have any corresponding dfn in the source or "the <span>XML syntax</span>" which to match the right dfn should be <span>the XML syntax</span> or <span data-x="the XML syntax">XML syntax</span>
B. ignore non-xref spans like "<span class="note">…</span>"

So starting by only checking for span elements that have no attributes, we can skip the B instances.

But other than the span-for-xref case I can’t think of any similar uses of particular elements in the source where for some reason we might want to find instances of them that have no attributes at all.

[annevk]

In current tip of tree, lines 241 and 1447 also do this check, both related to attribute counting. I'm not sure at what point it's worth having an abstraction, but I don't think this is quite it. (And if anything, this might more argue for some counting abstraction.)

[zcorpan]

So something like this will happily be converted to a link:

<span class="">The XML syntax</span>

Maybe we should switch to <a> instead?

[annevk]

True, we could probably exclude <span> elements that have attributes that are not data-x or switch to <a>. Seems like a follow-up though given the work involved and the PR provided has actually helped spotting a number of errors.

[zcorpan]

Yes, agreed. Filed #38