Miscellaneous editorial fixes #388
Conversation
|
The color coding somehow has not been added to all features dispositions. Affected are:
|
Links to the following clauses seem to be missing (assuming only normative constraints are considered):
In my opinion the links are sometimes a bit confusing e.g. |
The second feature in the |
There was a problem hiding this comment.
Have added some further comments additional to those raised by @spoeschel, which I haven't duplicated, but agree with.
imsc1/spec/ttml-ww-profiles.html
Outdated
| </tr> | ||
|
|
||
| <tr> | ||
| <td><code>#timing</code></td> | ||
|
|
||
| <td colspan="2">permitted</td> | ||
| <td colspan="2" class="permitted">permitted</td> |
There was a problem hiding this comment.
There's a recommendation (using SHOULD conformance language) against #timing in 7.12.17 that should be linked here.
| @@ -4356,9 +3827,12 @@ <h3>General</h3> | |||
| <section class='appendix'> | |||
| <h3>#progressivelyDecodable</h3> | |||
|
|
|||
| <p class="deprecation">The <code>#progressivelyDecodable</code> feature is designated as <em>permitted-deprecated</em> in the | |||
| profiles defined by this specification.</p> | |||
There was a problem hiding this comment.
I think this change is intended to address #376 but that issue proposes adding the deprecation warning to §7.8.2 where itts:progressivelyDecodable is specified. I propose adding it in both places.
| <td><code>#font</code></td> | ||
|
|
||
| <td colspan='2'>prohibited</td> | ||
| <code>#extent-image</code>, and <code>#extent-measure</code>.<br> |
There was a problem hiding this comment.
The TTML2 #extent-version-2 feature now also includes #extent-root-version-2 which further includes #extent-auto-version-2. I think we need to be explicit about the handling of those here, even though there's a blanket prohibition on TTML2 features not mentioned.
There was a problem hiding this comment.
This PR is not intended to cover the recent refactoring of features in TTML2. This PR is intended to be an editorial pass assuming nothing changed in TTML2. A future PR will address the refactoring of features in TTML2.
There was a problem hiding this comment.
I have opened #390
| @@ -71,6 +71,9 @@ | |||
| .inline-note {font-size: small} | |||
| .deprecation {background-color: lavender; border: 3px double; margin: 2px;} | |||
| .deprecation::before {content: "\26A0"; margin:0.2em; font-size:2em; float:left;} | |||
| td.permitted::first-line { color: green } | |||
| td.prohibited::first-line { color: #ff6666 } | |||
| td.permitted-deprecated::first-line { color: orange } | |||
There was a problem hiding this comment.
Have you checked the contrast of this colour against white, for accessibility?
There was a problem hiding this comment.
I have not checked. These colors/styles are a placeholder at this point.
There was a problem hiding this comment.
I have opened #391
|
|
||
| <td colspan='2'>prohibited</td> | ||
| <td colspan="3" style="text-align:center"><em>Relative to the TT Feature namespace</em><br> | ||
| All features specified in [[TTML2]] are <em>prohibited</em> unless specified otherwise below</td> |
There was a problem hiding this comment.
I'd like to review this approach with the WG, in case there's a strong view that we should list all features explicitly. Also, it should read "introduced" rather than "specified".
There was a problem hiding this comment.
All features specified in TTML2 (including features carried over from TTML1) that are not specified in the table are prohibited
There was a problem hiding this comment.
When prohibited features are no longer contained in the table, it could be difficult during standardization to tell if a new TTML feature is prohibitied in IMSC or has not yet been considered in IMSC.
There was a problem hiding this comment.
IMSC 1.1 reference TTML 2.0 explicitly, so there cannot be confusion since the list of features in TTML2.0 will not change.
OK, this makes sense, as the table header now states that unlisted features are prohibited. |
In 7.8.2, a similar box is required as well. |
|
The Working Group just discussed
The full IRC log of that discussion<nigel> Topic: Miscellaneous editorial fixes imsc#388<nigel> github: https://github.com//pull/388 <nigel> Nigel: Given that Stefan and I have given feedback, what do we need to discuss in the meeting? <nigel> Pierre: I'd like to walk through your feedback Nigel so we can close this and move on. <nigel> .. It's important to merge this pull request to allow other work to proceed. <nigel> Pierre: You've requested that features link back to constraints. <nigel> Nigel: Yes, based on whether there are conformance keywords, rather than just limiting <nigel> .. to SHALLs and SHALL NOTs. <nigel> Pierre: Okay, I can deal with that. <nigel> .. There a bunch of things that you and Stefan noted that are related to changes in TTML2 <nigel> .. but this pull request is not intended to address those. I want to deal with the TTML2 <nigel> .. feature refactoring separately, and not address https://github.com//pull/388#discussion_r195347251 <nigel> .. here. <nigel> Nigel: Okay, have we got an issue open for it? <nigel> Pierre: We can open an omnibus issue - it's next on my list. <nigel> Nigel: Sure, I don't mind taking that approach to allow this to continue. <nigel> Pierre: [colour contrast question] <nigel> Nigel: If the answer is "no" you haven't checked then fine, put that on and raise an issue <nigel> .. to deal with it, then we can proceed. <nigel> Pierre: Okay, I can do that. <nigel> Nigel: [TTML2 prohibition comment] I think you mean introduced rather than specified? <nigel> Pierre: No, really everything in TTML2. I think we should omit prohibited things. It's been <nigel> .. a source of errors when we tried to track features in TTML2 so it makes it easier to <nigel> .. maintain if we just include features with some support, and easier to read. <nigel> Nigel: I think we need to include all dependent features that are potential parts or related <nigel> .. to bigger "group" features even if they are in fact prohibited, just for clarity. The <nigel> .. `#extent-auto-version-2` comment above is a good example. <nigel> Pierre: That makes sense, that will be done. <nigel> Nigel: We can move that into the TTML2 feature change mop-up issue? <nigel> Pierre: Yes, absolutely. <nigel> i/Nigel: I think/Nigel: [TTML2 features comment] <nigel> Pierre: I wanted to make sure that we didn't think that excluding prohibited features is a <nigel> .. bad idea. That is the major substantial change in this pull request. <nigel> Nigel: Just testing the idea. Is it clear and unambiguous? Yes. <nigel> .. Is it helpful for implementers? Not sure. <nigel> Pierre: That's not clear to me either. <nigel> .. For authors it's much easier. <nigel> Cyril: It's easier from an editor's point of view. <nigel> .. I think I'm fine with that. <nigel> Nigel: One more point - the deprecation warning on ittp:progressivelyDecodable. <nigel> Pierre: I'll add that. <nigel> .. If I make those changes we discussed will you be able to approve them early tomorrow your time Nigel? <nigel> Nigel: Yes, I don't see why not. <nigel> SUMMARY: Pierre to make changes as discussed. |
|
@spoeschel Per #390, I plan to do a complete pass in a separate PR to address changes made by TTML2 CR2 |
@nigelmegitt I tried to implement this, and it didn't really work since this means some prohibited features are included in the table, and some are not, which is even more confusing. I am open to suggestions, or just leaving as is. |
|
@palemieux how about noting that the prohibited related features are prohibited (and listing them) as a note under the features to which they are related, rather than including them in the main table? For example:
|
|
@palemieux: However strictly speaking this is already the case with this MR, as features that are prohibited in only one of the two profiles are still in the table (they have to, of course). Maybe it would make sense to add a list (in prose) after the TTML table, that contains all features that are prohibited in both IMSC profiles. |
@spoeschel The challenge is making sure that list is up-to-date. Otherwise, what happens to a feature that is not included in the table? It is permitted, or prohibited? This actually happened with |
This will burden the table significantly:
In fact, instead of listing dependent features, I am wondering if a simply link to the feature in TTML2 should be provided. |
@spoeschel , @palemieux already made the point that maintaining the list of prohibited features is an onerous editorial task, and adding such a list would not help. I think a middle way to mention the related prohibited features by note, locally, and not try to list them all in prose, would be better. |
|
@nigelmegitt Having tried a couple of options, I feel that the current approach, i.e. listing dependent features but not stating their individual disposition, is the least terrible: the reader does not have to jump to TTML2 to determine dependent features, and can easily determine whether a feature is supported by IMSC 1.1 by searching the document for each of the dependent features. Maybe there is a smarter approach, but I have not seen it yet. |
@palemieux I wondered that too; it certainly has the merit of being easy to traverse to the definition. |
@palemieux right, but then the list of dependent features needs to be complete, yes? |
There was a problem hiding this comment.
Approving now so that the editor can merge this and make further progress. If you do that @palemieux but there are outstanding comments not yet addressed please could you raise issues for those comments, so we continue to track them? I'm conscious that this is blocking you at the moment, so want to unblock it as per your request.
Good point; I see that list wouldn't be a good option.
This would make life definitely easier. |
|
After separating the two (permitted) provisions for Otherwise it seems fine to me for now. |
|
@spoeschel I have opened #393 |
palemieux
deleted the
issues-359-363-374-375-376-380-reformat-features-table+misc-editorial-issues
branch
Reformat features table (#363)
Add link to constraints in features table (#359)
Fix #background disposition (#374)
Misc editorial fixes (#375)
Add deprecation warning to ittp:progressivelyDecodable (#376)
Add link to constraints in features table (#380)