Skip to content

Conversation

silviapfeiffer
Copy link
Member

Addresses #375

index.bs Outdated
<h3 id="video-viewport">Video viewport</h3>

<p>The <dfn lt="video viewport">video viewport</dfn> is the video's rendering area which in the case
of HTML5 is defined by the |video|'s <a>concrete object size</a>.</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 href for the concrete object size link is https://drafts.csswg.org/css-images-3/#concrete-object-size but the spec is on /TR at https://www.w3.org/TR/css3-images/#concrete-object-size - any reason not to use the latter?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what bikeshed picked for us. I'll added an explicit ref note so autolinks for preparing the /TR spec will link to the /TR spec.

@silviapfeiffer
Copy link
Member Author

@nigelmegitt could I get a review on this PR?

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.

Complex to review, thanks for your patience. There's one link I found that points to an ED when there's a CR.

Looking for places where video is used when video viewport is meant, I found instances on lines 921, 938, 967, 1074, 1082, 1196, 1205, 1212, 1387, 1964, 4552, 4555, 4601, 4694, 4700, 5980, 6004 and 6010 of this pull request branch. Since the key issue being fixed here is confusion between video and video viewport, please could you step through those and fix them @silviapfeiffer ? You might think there's a good reason for some of them to be the way they are, but I couldn't spot one.

<h3 id="video-viewport">Video viewport</h3>

<p>The <dfn lt="video viewport">video viewport</dfn> is the video's rendering area which in the case
of HTML5 is defined by the |video|'s <a spec=css-images-3>concrete object size</a>.</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 concrete object size link points to the CSS3 images ED rather than the CR at https://www.w3.org/TR/css3-images/#concrete-object-size where the definition is the same.

@silviapfeiffer
Copy link
Member Author

Thanks for taking the time for this in-depth review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants