Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify missing validation semantics (#362, #363, #367, #541). #594

Merged
merged 13 commits into from Feb 19, 2018

Conversation

skynavga
Copy link
Collaborator

Closes #362.
Closes #363.
Closes #367.
Closes #541.

@skynavga skynavga self-assigned this Jan 28, 2018
@skynavga skynavga added this to the Editor's CR Work List milestone Jan 28, 2018
@nigelmegitt
Copy link
Contributor

Built version at https://rawgit.com/w3c/ttml2/issue-0362-0363-validation-build/index.html (note that schemas are also modified by this pull request)

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm generally supportive of the direction here, but not happy with a few of the details, listed in the comments.

<loc href="#terms-effective-validation-profile">effective validation profile</loc> is the
<loc href="#terms-effective-content-profile">effective content profile</loc>;</p></item>
<item><p>otherwise, the <loc href="#terms-effective-validation-profile">effective validation profile</loc> is the
<loc href="#terms-effective-processor-profile">effective processor profile</loc>.</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean to specify that a processor profile is used for validating a document instance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same thing as it means when using content profile: find a schema from profile, then validate

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work - the spec talks about inferring a processor profile from a content profile in lots of places, but never the other way around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you seem to assume this is trying to infer a content profile, but that is not the case; it merely says if there is no content profile, then use processor profile to find an associated schema (since the new ttp:schema{s} elements can be put into a processor profile as well, or associated by the processing context);

the issue here is how to perform validation when no content profile is specified, e.g., on TTML1 documents; TTV, e.g., does this by making use of a built-in (code) association of particular schemas with the TTML1 profiles;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments attached to later lines in the file; I do not see any agreement from the WG to add ttp:schema{s}. The closest I can find based on an issue search is #435 (comment) which does not mention adding new syntax (and anyway that issue appears not to be the target of this pull request). In my view this is a request for a new feature, and we have certainly agreed not to do that, so it should be raised as an issue against ttml.next.

To resolve this comment though, I think we need somewhat loose words that recommend that the processor infer any validation rules such as schemas based on the processor profile, without specifying an algorithm for doing so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt I am not following you. Let me explain my premises, which perhaps will allow some clarity:

  1. to validate, you need to a schema (or an equivalent) to be associated with the document (at least one such mechanism is mandated in 3.1 (3); however, we did not define a standard mechanism in TTML1);
  2. we support multiple schema languages, so we can't simply choose the XSD schema association mechanism (xsi:schemaLocation, etc.), that is, we need something more generic;
  3. we already have a (rich) mechanism for associating one or two profiles with a document, either just a process profile or both a content profile and a processor profile;
  4. therefore, it makes sense to associate (one or more) schemas with a profile in order to satisfy (1) and (2) above (indeed, this is what TTV does internally, but without the benefit of an author defined association mechanism - that is, TTV has a hardwided set of associations from a "model" to a set of profiles and sets of schemas for these profiles);
  5. an author may specify content profile for a document, which should take priority over determining schemas, in which case schemas associated with the effective processor profile for such a document are ignored;
  6. if an author does not specify content profile it is null, in which case the effective processor profile may supply such schema associations;

If we do not specify something like the proposed ttp:schema element, then we are left with no standardized association mechanism whatsoever, which, in my mind, renders the validation functionality of TTML2 into horribly underspecified state such that TTML2 cannot claim to support validation at all.

Now, I don't have any problem with allowing an implementation to look elsewhere for schema associations (i.e., to look beyond any author specified schemas explicitly defined using ttp:schema), but I have a serious problem with not providing at least one standard way to formally establish an association.

Also, I see no problem of logic or processing with respect to using associations obtained from a process profile as a backup for when an author does not specify a content profile.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt although my preference is to require support for ttp:schema(s) via #validation, I am willing to move this support to an independent #validation-schemas feature designator, which, if we take this option, that leaves only implied (implementation dependent) association between the effective validation profile and any potential schemas in the absence of support #validation-schemas; it is also my intention that we add these elements to our standard profiles and point at the RNC and XSD schemas we define;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to validate, you need to a schema (or an equivalent) to be associated with the document

@skynavga This is false. A validation processor does not require a schema.

it makes sense to associate (one or more) schemas with a profile in order to satisfy (1) and (2) above

Even if this is true, the group has not discussed it and no case has been made that there is a pressing need for it to be added now.

TTML2 cannot claim to support validation at all

I disagree with this - if it were true then it would equally have been true for TTML1. Yet there are validation processors for TTML1 documents and leaving the implementation mechanism for validation to the implementer has worked fine so far.

I am willing to move this support to an independent #validation-schemas feature designator,

No, sorry, this is a new feature and it has to be deferred. We are too late to have this in TTML2 now. Giving the feature a designator does not mitigate the fact that there is no consensus to add the feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now addressed since the ttp:schema and ttp:schemas additions have been removed.

spec/ttml2.xml Outdated
<def>
<olist>
<item><p>if a document's <loc href="#terms-effective-validation-profile">effective validation profile</loc> is associated with one or more
schemas, then select the <loc href="#terms-effective-validation-schema">effective validation schema</loc> from this set based upon the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This suggests selecting a single validation schema, which to be useful requires that all the schemas are combined into a single schema document. I have seen common practice of applying multiple validation schemas separately when validating a document instance, rather than being required to combine all the provisions of all of them. Can we permit validation against all applicable schema rather than selecting one? Possibly by permitting synthesis of a single schema.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will tweak to read "select appropriate schema(s)" or something that is equivalent to this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

spec/ttml2.xml Outdated
<def>
<olist>
<item><p>if validation is disabled by a <loc href="#terms-higher-level-protocol">higher level protocol</loc>
or if the computed value of the <att>ttp:validation</att> parameter is <code>prohibited</code>, then exit this procedure;</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use case for allowing a document instance to prohibit validation? If I were validating incoming document instances I would always override this. I do not think the option should be available. "optional" is as strong as a document instance should be able to specify. At the very least it must be possible for a higher level protocol to override ttp:validation="prohibited".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feature (prohibited) is not new, so we aren't going to revisit it in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous text included:

unless the end-user or application overrides this prohibition

which appears to have been removed. That needs to be added back in here in some form.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, can do

spec/ttml2.xml Outdated
<p>recognizes and is capable of transforming the following element vocabulary defined by <specref ref="profile-element-vocabulary"/>:</p>
<ulist>
<item><p><loc href="#profile-vocabulary-schemas"><el>ttp:schemas</el></loc></p></item>
<item><p><loc href="#profile-vocabulary-schema"><el>ttp:schema</el></loc>;</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too open-ended since it is not qualified by a minimal list of schema type values that is supported. I would suggest that we remove the requirement for schema processing from #validation and create new feature designators for #schema-xsd and #schema-rnc that require support for schemas and schema qualified by the appropriate type.

This is to allow a validating processor to be created specific to a particular profile without defining how that validation must be implemented, and to allow it to validate using a schema without the document instance specifying that schema.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. We aren't prescribing image types, so we don't need to do so with schema types. The only reason to support the new ttp:schema elements is to support validation, so it needs to be tied to the #validation feature.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of the issues linked from this pull request appears to concern the addition of schema validation so in the absence of evidence that we have consensus to add this feature it needs to be removed from this pull request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which feature you referring to? the #validation feature was not added in this PR

Copy link
Collaborator Author

@skynavga skynavga Jan 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt just to be clear, requiring support for the ttp:schema{s} element types does not imply support for any particular schema validation whatsoever; it just means you support the association of schema(s) with a profile.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which feature you referring to?

@skynavga schema and schemas, which appear to be introduced in this pull request, but for which I can see no proposal or request in the linked issues.

Copy link
Contributor

@palemieux palemieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skynavga @nigelmegitt Can one of you describe the problem(s) that this PR attempts to solve. The reference tickets do not provide background.

@skynavga
Copy link
Collaborator Author

@palemieux this PR addresses three long standing open issues (#362, #363, and #367) as well as a more recently posted issue (#541); without this PR, the semantics of ttp:validate and ttp:validateAction are not well defined, as there was no defined procedure for [validate document]. As part of satisfying this, at least one standard mechanism is needed to satisfy "is or can be associated with one of the abstract document types defined by 4 Document Types" as found in Section 3.1, item (3). This is the purpose of the schemas/schema element children of ttp:profile.

@palemieux
Copy link
Contributor

@skynavga What is the use case for ttp:validation? Validation is not something that should be under the authors control.

@skynavga
Copy link
Collaborator Author

@palemieux I don't know how to respond to such a claim. Of course validation is and should be under authorial control. What is the point of specifying a content profile? The validation features defined in TTML2 are and have been one of its key features from the very first stages of designing and specifying TTML2.

Copy link
Contributor

@palemieux palemieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very extensive change to the validity model, that we had just finished reviewing in the context of TTML1. I believe it is too risky to include this close to CR, and does not seem to fix a practical problem. If this is going to be accepted, I think the submitter needs to walk the group through the change and the motivation behind it.

@palemieux
Copy link
Contributor

Of course validation is and should be under authorial control. What is the point of specifying a content profile?

The point of specifying a content profile is let other entities validate the document. The author does not need to provide any additional information. In other words, I do not see the need for ttp:validate. Can you provide a concrete workflow/use case? Surely you have thought of one?

@skynavga
Copy link
Collaborator Author

@palemieux The only "change" here is

  1. Defining a [validate document] procedure; and,
  2. Defining a formal way to associate schemas with documents (via a profile).

This is not new, and it is assumed from the semantics of ttp:validate and ttp:validateAction and has long been documented. As you very well know, a parameter must be normatively used in some processing to be called a parameter. This PR formally provides that use.

However, perhaps you mistake my meaning of "control" here. Here, I mean having the ability to express authorial requirements or at least desires with respect to validation. Of course, the ultimate control over whether validation is performed and what actions to take is the domain of the implementation and the end user, so the latter can clearly override the author. However, the author needs to be permitted to provide guidance to this process, thus the ttp:validate and ttp:validateAction parameters.

@skynavga
Copy link
Collaborator Author

@palemieux first, to correct a point, we should be talking of ttp:validation and ttp:validationAction (I used the wrong names above); next, in usage contexts where content validity is important or essential, then an author may wish to insist that a document only be processed if it is determined to be valid by the receiving entity, in which case, the author would

  1. specify a processor profile that requires support for ttp:validation;
  2. specify ttp:validation='required';

Step (1) ensures that the document is only processed by a processor that is a validating processor, and (2) ensures that validation is actually performed. If an implementation does not support validation, then (1) would cause abort processing. If an implementation does support validation, but it is turned off, then (2) would cause abort processing. If an implementation does support validation and it is turned on, then (2) would cause validation to take place, in which case ttp:validationAction determines the action taken when a validation error or warning occurs.

This is already implemented, so there is no risk for CR1.

@skynavga
Copy link
Collaborator Author

I would also remind folks that we already define and or make reference to the following going back to the first WD of TTML2 (12 February 2015) [1] (and earlier [3]):

  • performing validation with respect to effective content profile
  • validating content processor (marked as TBD)
  • validating processor with respect to deprecation and obsoletion processing
  • perform validation under [construct effective content profile]
  • ttp:validation
  • ttp:validationAction
  • many other instances which I won't attempt to enumerate here

The current (new) TTML2 profiles and validation features can mostly be traced back to [2], which were first implemented in TTML2 in May 2014 [3].

The only thing new in this PR that isn't part of this earlier work is the addition of the ttp:schema[s] elements, which are important to realize effective semantics for satisfying the following editor's notes added one week after posting [3]:

screen shot 2018-02-10 at 3 37 59 pm

screen shot 2018-02-10 at 3 38 25 pm

[1] https://www.w3.org/TR/2015/WD-ttml2-20150212/
[2] https://dvcs.w3.org/hg/ttml/raw-file/default/ttml2/design/TPAC2013-TTMLProfiles.pdf
[3] https://lists.w3.org/Archives/Public/public-tt/2014May/0111.html

@palemieux
Copy link
Contributor

the addition of the ttp:schema[s] elements

Schemas are not sufficient to validate the document, so why even include them.

@skynavga
Copy link
Collaborator Author

skynavga commented Feb 10, 2018

@palemieux they may not be sufficient, but they are necessary; and, furthermore, we go out of our way to specify two realizations

@skynavga
Copy link
Collaborator Author

@nigelmegitt @palemieux I have attempted to address all comments; for diff from the original PR, see 5c8b527

@nigelmegitt
Copy link
Contributor

@skynavga Sorry, no, you have not addressed the point that we cannot introduce schema and schemas at this time, and that needs:

  1. An issue
  2. Consensus to proceed

Such an issue must, at this time, be targeted at ttml.next and so must any pull request that introduces the feature. We do not have time to go ahead with it in TTML2 now.

This pull request needs to be modified to revert the addition of schema and schemas. We're just too late for this one. This is a process point. The alternative is that we defer all of the issues addressed in this pull request because there's no acceptable pull request that addresses them.

@skynavga
Copy link
Collaborator Author

skynavga commented Feb 12, 2018 via email

@skynavga
Copy link
Collaborator Author

@cconcolato please review

spec/ttml2.xml Outdated
<p>is a <loc href="#terms-validating-content-processor">validating content processor</loc>;</p>
</item>
<item>
<p>recognizes and is cable of transforming the following attribute vocabulary defined by <specref ref="profile-attribute-vocabulary"/>:</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: cable -> capable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor

@cconcolato cconcolato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR is quite large and I'm not sure it's necessary, but given that it's behind a feature designator that one can prohibit in a profile, that the the new schema/schemas elements have been removed, I'm approving it.

@palemieux
Copy link
Contributor

Is it accurate that [validate document] only depends on schema validation, i.e. does not include any semantic validation not included in the schema validation?

@nigelmegitt
Copy link
Contributor

This hasn't built properly. Going to do some fiddling.

@skynavga
Copy link
Collaborator Author

skynavga commented Feb 16, 2018 via email

@nigelmegitt
Copy link
Contributor

@skynavga I'm seeing a note to that effect at line 995 already, but not in the built version. I'm going to close and reopen to try to trigger a build.

@nigelmegitt nigelmegitt reopened this Feb 16, 2018
@palemieux
Copy link
Contributor

@skynavga Validation should be against the entire specification. I do not understand why validation needs to be tied to schemas, and then expand to term schema to mean the entire specification. Anyway, I guess this could be fixed post-CR1.

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the semantics of a document instance preventing or bypassing validation described here are completely inappropriate, but will not block immediately pending the answer to my request for a use case. Right now I think we should not merge this, and just take out the issue notes.

spec/ttml2.xml Outdated
<p>Validation is an optional process that may occur when a candidate <loc href="#terms-document-instance">document instance</loc> is
consumed or otherwise processed by a <loc href="#terms-validating-content-processor">validating content processor</loc>. If such processing
takes place and an exceptional condition occurs, then processing may be aborted, as described in further detail below.</p>
<p>If processing is aborted during <loc href="#terms-validating-content-processor">validating content processor</loc>, then the possible side effects
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grammar issue: "If processing is aborted during validating content processor, ..." doesn't make sense. Should this say "If processing is aborted during validation, ..."?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing now.

<item><p>only part of a non-conformant document may be presented by a <loc href="#terms-presentation-processor">presentation processor</loc>,
resulting in an incomplete presentation.</p></item>
</ulist>
<p>If abort processing does occur as a side effect of validation processing, then the location in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why "side effect"? Surely it is a direct effect. Suggest removing "side" here., or even replacing "side effect" with "consequence" or "result".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

spec/ttml2.xml Outdated
<glist>
<gitem id="semantics-validation-state-validation-exception-message">
<label>message</label>
<def><p>An arbitrary (implementation defined) string the describes or characterizes the exception</p></def>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/the/that

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing now.

<item><p>if a <termref def="defs-validation-error">validation error</termref> condition is detected, then perform the semantics that apply
to the computed value of the <loc href="#profile-attribute-validationAction"><att>ttp:validationAction</att></loc> parameter property;</p></item>
<item><p>if a <termref def="defs-validation-warning">validation warning</termref> condition is detected, then perform the semantics that apply
to the computed value of the <loc href="#profile-attribute-validationAction"><att>ttp:validationAction</att></loc> parameter property.</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two mechanisms present by which a document instance can attempt to wriggle out of validation: firstly by setting ttp:validation to prohibited and secondly by setting ttp:validationAction to warn or ignore. Both of these options have no place in a document instance. If a system wishes to validate a document instance then it is up to the operators of that system what behaviour should apply, not up to the author of the document instance being provided to it.

@skynavga can you give any use case where this would be helpful? I

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, setting ttp:validationAction to warn or ignore does not prevent validation. The ttp:validationAction property is used only for the purpose of determining how to process a violation, not whether to detect a validation. As for having an author set ttp:validation to prohibited, we have already specified that the end-user or application may override this prohibition, so the prohibition from the author is merely advisory in an absolute sense. However, there is nothing wrong with an author saying that validation should not occur if it is there interest to do so. We cannot dictate their intended use with respect to a document.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt a use case for an author specifying ttp:validation='prohibited' may include any of the following reasons:

  • the content is of low value and the author doesn't care if invalidity causes it to fail; e.g., the author may not wish to take the time to validate the content in the first place and is willing to let the client do whatever it can with it;
  • the content is known to be invalid (for whatever reason and the author would like a validating client to take no action with respect to validation;

Isn't this the model most used on the Internet today in general?

We have defined the default for ttp:validation to be optional, and we have stated that the application (client) or end user can override ttp:validation='prohibited'; therefore, it is truly up to the client to decide whether to validate or not. In that sense, you can distinguish between optional and prohibited as nothing more than a hint on the part of the author as to what they prefer. However, the semantics of this are well defined as far as the stated mechanisms are concerned: you haven't shown they are not implementable!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skynavga I'm going to record my thought on this response here, and then move on.

  • the content is of low value and the author doesn't care if invalidity causes it to fail; e.g., the author may not wish to take the time to validate the content in the first place and is willing to let the client do whatever it can with it;

The author does not need to specify anything in the document for this scenario - they merely need to have some idea that the target player(s) will be able to do something reasonable.

  • the content is known to be invalid (for whatever reason and the author would like a validating client to take no action with respect to validation;

This is actively dangerous. As far as I can tell, this scenario (almost) never be a good idea - clients deliberately providing invalid data and then telling the downstream chain to pass it is obviously broken except in the specific case of test scenarios where player behaviour in the presence of invalid documents needs to be checked.

Isn't this the model most used on the Internet today in general?

No, I've never seen data instruct validators what they should do. I think it is the opposite of the model most used, which is rather that validators are configured with a specific set of criteria out of band from the data being validated. The closest to it is where an XML document specifies a DTD or XSD and a general purpose validator fetches that and uses it for validation purposes - but that is a different case.

you haven't shown they are not implementable!

That isn't the right question - the first question is, would it be a good idea?!

I still think this feature is fundamentally misguided, but on the basis that it is behind an at-risk feature designator, and in the interests of making progress, I'm going to review as though it were acceptable.

@skynavga
Copy link
Collaborator Author

skynavga commented Feb 16, 2018 via email

@skynavga
Copy link
Collaborator Author

skynavga commented Feb 17, 2018 via email

Copy link
Contributor

@palemieux palemieux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ttp:schema{s} attributes are removed. I think that validation and schemas still need to be disentangled, but that can wait after CR me thinks.

Copy link
Contributor

@nigelmegitt nigelmegitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remain unconvinced (actually, quite concerned) about some aspects of this but will remove my Request Changes review to allow progress. Some of the comments are addressable with a quick edit.

<item><p>if a <termref def="defs-validation-error">validation error</termref> condition is detected, then perform the semantics that apply
to the computed value of the <loc href="#profile-attribute-validationAction"><att>ttp:validationAction</att></loc> parameter property;</p></item>
<item><p>if a <termref def="defs-validation-warning">validation warning</termref> condition is detected, then perform the semantics that apply
to the computed value of the <loc href="#profile-attribute-validationAction"><att>ttp:validationAction</att></loc> parameter property.</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skynavga I'm going to record my thought on this response here, and then move on.

  • the content is of low value and the author doesn't care if invalidity causes it to fail; e.g., the author may not wish to take the time to validate the content in the first place and is willing to let the client do whatever it can with it;

The author does not need to specify anything in the document for this scenario - they merely need to have some idea that the target player(s) will be able to do something reasonable.

  • the content is known to be invalid (for whatever reason and the author would like a validating client to take no action with respect to validation;

This is actively dangerous. As far as I can tell, this scenario (almost) never be a good idea - clients deliberately providing invalid data and then telling the downstream chain to pass it is obviously broken except in the specific case of test scenarios where player behaviour in the presence of invalid documents needs to be checked.

Isn't this the model most used on the Internet today in general?

No, I've never seen data instruct validators what they should do. I think it is the opposite of the model most used, which is rather that validators are configured with a specific set of criteria out of band from the data being validated. The closest to it is where an XML document specifies a DTD or XSD and a general purpose validator fetches that and uses it for validation purposes - but that is a different case.

you haven't shown they are not implementable!

That isn't the right question - the first question is, would it be a good idea?!

I still think this feature is fundamentally misguided, but on the basis that it is behind an at-risk feature designator, and in the interests of making progress, I'm going to review as though it were acceptable.

<p>A <loc href="#terms-content-processor">content processor</loc> which performs validation processing on a
TTML <loc href="#terms-document-instance">document instance</loc> prior to performing any other type of processing.</p>
<p>A <loc href="#terms-content-processor">content processor</loc> which does or can perform validation of a
candidate <loc href="#terms-document-instance">document instance</loc> prior to performing certain other type of processing.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ordering constraint here "prior to performing" is incorrect: since no specific abort point is required, it is feasible and reasonable for a processor to interleave validation with other processing arbitrarily. It should simply be "in addition to performing".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raised as #674.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If validation occurs then it should be performed in a context where any content that might be validated is not otherwise used until validation is completed on that content. If an implementation wishes to interleave validation and post-validation content processing, that is an implementation decision. I will add an informative note to this end via #674.

declared to apply to the <loc href="#terms-document-instance">document instance</loc>; or</p></item>
<item><p>a <termref def="defs-validation-error">validation error</termref> occurs when performing the
<loc href="#semantics-procedure-validate-document"><phrase role="strong">[validate document]</phrase></loc> procedure.</p></item>
</olist>
</item>
<item>
<p>The processor supports all mandatory processing semantics defined by this specification.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line now seems to be a problem: if a document instance specifies a content profile that prohibits something that is Mandatory in E.2 (like #region-timing or #time-offset) then the processor is apparently not a generically conformant processor even though it is a subset. Shouldn't the requirement be that the processor supports all processing semantics defined by this specification and permitted by the applicable profile?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raised as #673.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nigelmegitt regarding

if a document instance specifies a content profile that prohibits something that is Mandatory in E.2 (like #region-timing or #time-offset) then the processor is apparently not a generically conformant processor even though it is a subset.

you are confusing (conflating) the role of a content and processor profile; the language we are discussing only applies to processor profiles and the semantics of processing a document according to a processor profile; furthermore, there is no conflict in having a content processor prohibit the USE of a feature while at the same time using a processor profile that requires SUPPORT of the feature

as far as I can tell, there is no problem with the current text, which is merely an editorial rewrite of the TTML1 text

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, my comment wasn't worded well. The point is that #region-timing and #time-offset are mandatory features for generic processors but are likely to be specified as prohibited by some content profiles. In that sense the line is too strict - it requires processor support for something that in some cases is not in fact needed.

One possible resolution is to make those features optional rather than mandatory.

A second possible resolution is to say that generic conformance requires support for the intersection of a) those features specified as mandatory for generic processors and b) those features specified as required by the (possibly inferred) processor profile.

A third possible resolution would be to exclude mandatory features from those that may be prohibited by a content profile.

<item><p>only part of a non-conformant document may be presented by a <loc href="#terms-presentation-processor">presentation processor</loc>,
resulting in an incomplete presentation.</p></item>
</ulist>
<p>If abort processing does occur as a side effect of validation processing, then the location in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

<loc href="#terms-effective-validation-profile">effective validation profile</loc> is the
<loc href="#terms-effective-content-profile">effective content profile</loc>;</p></item>
<item><p>otherwise, the <loc href="#terms-effective-validation-profile">effective validation profile</loc> is the
<loc href="#terms-effective-processor-profile">effective processor profile</loc>.</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now addressed since the ttp:schema and ttp:schemas additions have been removed.

@@ -4663,11 +4811,15 @@ ttp:validationAction
</tbody>
</table>
<p>If this parameter's value is <code>abort</code>, then, a <loc href="#terms-validating-content-processor">validating content processor</loc>
must abort processing of a TTML <loc href="#terms-document-instance">document instance</loc> when validation processing fails.</p>
must abort processing of a TTML <loc href="#terms-document-instance">document instance</loc> when a
<termref def="defs-validation-error">validation error</termref> exception occurs.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line appears to specify something in conflict with the statement in §5.3 that the processor is permitted to continue, and the moment when processing is aborted is unspecified. Suggest changing "when" to "if".

Also, the document processing context must be able to override this. For example, consider a transformation processor whose role is to produce a valid output TTML2 document instance for any given input document claiming to be a TTML2 document instance, including ones that would fail validation. This task must by definition validate the input, but in the case of errors that it is able to fix it must not abort before completing its task even if the document instance specifies ttp:validationAction="abort".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment raised as #672.

@nigelmegitt nigelmegitt dismissed their stale review February 19, 2018 13:54

Changes made subsequent to original "request changes" review.

@skynavga skynavga merged commit fefae58 into master Feb 19, 2018
@skynavga skynavga removed their assignment Feb 19, 2018
@skynavga skynavga deleted the issue-0362-0363-validation branch March 9, 2018 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants