modifying Schematron constraint in ab for #1988

[ebeshero]

This is a repair to the Schematron constraint in <ab> to resolve #1988. It's working with a test build on my branch.

[ebeshero]

@martinascholger @martindholmes @sydb I think this is ready to pull in to resolve #1988. Take a look and see if you agree?

[martindholmes]

Looks good to me! Thanks @ebeshero.

[ebeshero]

@sydb Looking at this again, I remember why the Schematron needs to test for ancestor::floatingText[ancestor:l]. The <ab> might be a descendant of an <l> that is itself a descendant of the <floatingText>, and be completely invalid in that case. Both the <l> and the <floatingText> could be ancestors of <ab>, but in the wrong order on the ancestor axis!

So, we absolutely need to make sure that the floatingText has an ancestor l in this Schematron test. Otherwise we could be allowing this:
//floatingText//l/ab , and that would violate the abstract model. Hope that makes sense.

I posted my test TEI file for this issue on the ticket: #1988 (comment)

[peterstadler]

Just a quick question:

• Shouldn't the Schematron constraint #abstractModel-structure-l on <p> be updated as well (as proposed by @martindholmes originally)?

and another proposal:

• Shouldn't we add @ebeshero 's test file (probably with respective checks for <p>) to the tests?

[lb42]

The schematron constraint should presumably apply to all members of model. PLike. But I don't know if the odd processors handle classSpec/constraintSpec correctly or at all.

[martindholmes]

@lb42 That's a really good question. I'm going to raise a ticket for the Stylesheets group to investigate it. We already have some problems with constraints defined at the attribute level, and in my own projects I make a point of putting all the Schematron at the schemaSpec level and using full contexts, but that can't work for something based on a model because the information about model members is not there in the instance document.

[ebeshero]

@peterstadler @martindholmes I think I lost track of the issue for <p> back on the original ticket! Shall I add it to this PR for now since presumably it'll work for the moment? We can maybe find a better solution later for checking at the level of the model or class when we investigate this with the Stylesheets group...

[ebeshero]

By "add it", I mean, check in and modify the Schematron constraint for <p> as @martindholmes indicated on the original ticket... That would complete the tiny set of elements (just <ab> and <p>) in model.pLike anyway.

[martindholmes]

@ebeshero That would solve the problem until we get around to looking at TEIC/Stylesheets#519.

[ebeshero]

Just noting that there's a similar constraint already on <p>, but it doesn't go far enough. Currently it just permits l//note//p, and it's rather differently set up.

<constraintSpec ident="abstractModel-structure-l" scheme="schematron">
    <constraint>
      <report xmlns="http://purl.oclc.org/dsdl/schematron" xmlns:tei="http://www.tei-c.org/ns/1.0" test="ancestor::tei:l[not(.//tei:note//tei:p[. = current()])]">
        Abstract model violation: Lines may not contain higher-level structural elements such as div, p, or ab.
      </report>
    </constraint>
  </constraintSpec>

I'll change that to make it more like the new rule to cover <figure> and <floatingText> too.

One more thing I discovered: Try nesting a <div> inside a <floatingText> within an <l> or a <p>. That's also an abstract model violation. So we need to do this again on <div>. (Do we care about div1 through div7)?

I wish we could just make one fresh constraintSpec on <l> to handle all of these cases in one rule, but I think I know the answer to that: we would not see the error firing on the trouble-making element that's violating the abstract model.

[ebeshero]

I've enlarged my test file a bit to test with <p>, and also poke around at various configurations of <floatingText>. Apparently I really don't need that predicate that I thought was necessary on floatingText, to check if it has an ancestor l. An l can't have an <ab> or <p> child simply because of the content model of l, and a floatingText as a distant ancestor (//floatingText//l/ab) doesn't change anything. (So I'm pulling out that predicate after all.)

[ebeshero]

The issue with <div> is easily corrected. The test for checking an ancestor::p or ancestor:ab was missing a necessary set of parentheses. So it was checking for //ab//floatingText//div but not //p//floatingText//div. Here's the problematic Schematron test:

ancestor::tei:p or ancestor::tei:ab and not(ancestor::tei:floatingText)

And it needs to be:

(ancestor::tei:p or ancestor::tei:ab) and not(ancestor::tei:floatingText)

(And it needed a test for the similar situation with l and floatingText).

[sydb]

I suspect we should just go with the changes @ebeshero has made here, for now, as we are only days away from freeze. But that said, I have an alternate solution that I think will work, and might lend itself quite well to being an abstract pattern, methinks. See
abstract_model_violation_checker.zip.

[sydb]

I tested against the test file with errors, and added a few more conditions. These did the right thing, at least in those cases. So while it may not be perfect, it does seem to be better than what we had.