Fix #2361: Use new Web IDL buffer primitives #2363

rtoy · 2021-06-28T22:01:04Z

Updates spec to use new Web IDL buffer primitives. The changes are
marked as candidate corrections, but we use several correction
amendments to make it easy to see what the new and old text will be.
Using just one makes it super-hard to figure out what controls the
text, and the text sometimes doesn't always clearly show what's
deleted and what's inserted because the links are colored correctly,
and struck-through text doesn't strike through links. (A bikeshed
issue?)

Preview | Diff

See whatwg/webidl#987.

Updates spec to use new Web IDL buffer primitives. The changes are marked as candidate corrections, but we use several correction amendments to make it easy to see what the new and old text will be. Using just one makes it super-hard to figure out what controls the text, and the text sometimes doesn't always clearly show what's deleted and what's inserted because the links are colored correctly, and struck-through text doesn't strike through links. (A bikeshed issue?)

rtoy · 2021-06-29T20:57:19Z

Biekshed has been updated to include the WebIDL references that were previously failing in https://travis-ci.com/github/WebAudio/web-audio-api/builds/230909490.

However, the link for "get a reference to the buffer source" is now no longer part of WebIDL, I think that means we should just remove the link for the <del> part since it doesn't work.

rtoy · 2021-07-02T15:34:26Z

index.bs

 		According to the rules described in <a href="#acquire-the-content">acquire the content</a>
-		either [=get a reference to the buffer source|get a reference to=]
+		either <del cite=#c2361-2>get a reference to</del><ins cite=#c2361-2>[=ArrayBufferView/write=]</ins> into


The phrasing for the new part is a bit confusing to me. The new part says

According to the rules described in acquire the content either write into...

But we're not writing anything at this point. getChannelData allows you to get a copy or write at some later time; we're not writing any data now.

Is this the phrasing we really want to use?

@domenic WDYT?

Yeah, I'm not sure I understood this part of the spec in its original formulation either; it didn't provide an algorithmic series of steps that explain what's going on. I'd be interested in seeing what the implementations do as an algorithm, so that we could make this spec precise instead.

To be clear, the existing spec didn't really make sense either, as it was trying to apply the "get a reference to"/"get a copy to" algorithm to [[internal data]], which is not a BufferSource.

Fair enough. I'll try to rewrite it a little so it seems less awkward.

rtoy · 2021-07-02T15:37:04Z

index.bs

+<h3 id="candidate-changes-1">
+  Candidate Changes since Recommendation of 17 Jun 2021
+</h3>
+  * <a href="https://github.com/WebAudio/web-audio-api/pull/2361">PR 2361</a>: Use new Web IDL buffer primitives


This correction used 7 correction boxes to make it easy to see that something was changed. Perhaps it would make sense to link to all 7 boxes here in the change log so there's an easy way to find all 7?

rtoy · 2021-07-07T16:04:21Z

index.bs

+		  <span class="marker">Candidate Correction
+		  <a href="https://github.com/WebAudio/web-audio-api/issues/2361">Issue 2361-1.</a>
+		  </span>
+		  Use new Web IDL buffer primitives


Since there are 7 related changes, it might be useful to add buttons to move to the next (and previous) corrections. This clutters things up a bit, but makes it easier to visit all the related changes. Or we can just expect the user to look at the changelog to find all the related corrections, where we can generate the list automatically.

Is it important to be able to jump between changes in c2361 group? If not, the button seems like a distraction.

If this is a new practice for all amendments across different specs, then you can ignore my comment above.

Without the buttons, finding related changes is harder. We do have the ChangeLog which has a list of the changes, but having to go back and forth to the change log makes looking at the changes harder.

See also #2366 (comment)

Not sure what you mean by "new practice" and what part of the change you're referring to.

Change "write" to "allow writing into" so reflects better what getChannelData allows.

rtoy · 2021-07-09T21:35:14Z

See #2366 for an example of adding previous and next buttons in each amendment box so it's easy to navigate each of the related changes. This won't show up in the preview. You'll have to build it yourself to see.

Fix up the script so bikeshed doesn't mangle it. Add an onload event listener so that when the doc is loaded we can run ListAmendments to add a list of the related changes for the change log.

…fer-prim-next-buttons

Adjust so bikeshed doesn't mangle it. Move the listener code into the same script segment so it's easy to find.

rtoy · 2021-07-12T16:21:38Z

Previous and Next buttons added. The JS code is now inline, so the preview shows the buttons and also the generated list of amendments for the changelog.

I'd appreciate help in getting the JS code nicer. This was just something I hacked together that worked for this case.

hoch · 2021-07-12T21:56:26Z

index.bs

@@ -97,6 +97,81 @@ window.addEventListener("DOMContentLoaded", function () {
 	});
 });
 </script>
+<script>


We have a separate amendments.js file above, yet it is being included directly in this HTML file. Is this intentional?

amendment.js should be removed since I moved all of that into index.bs so that the preview works.

Understood.

hoch · 2021-07-12T21:58:34Z

index.bs

+		  <span class="marker">Candidate Correction
+		  <a href="https://github.com/WebAudio/web-audio-api/issues/2361">Issue 2361-1.</a>
+		  </span>
+		  Use new Web IDL buffer primitives


Is it important to be able to jump between changes in c2361 group? If not, the button seems like a distraction.

hoch · 2021-07-12T21:59:50Z

index.bs

+		  <span class="marker">Candidate Correction
+		  <a href="https://github.com/WebAudio/web-audio-api/issues/2361">Issue 2361-1.</a>
+		  </span>
+		  Use new Web IDL buffer primitives


If this is a new practice for all amendments across different specs, then you can ignore my comment above.

rtoy · 2021-07-13T20:21:21Z

Also, take a look at https://pr-preview.s3.amazonaws.com/rtoy/web-audio-api/pull/2367.html#c2361-1 where the amendment box is scoped to include all the changes for the change. Works nicely for paragraphs, but for algorithms, I think the box needs to be around all the steps in the algorithm, so the box scope can be pretty big, as shown in 2361-1.

Alternatively, we could maybe just add amendment boxes in each step, since we're keeping track of related changes.

rtoy · 2021-07-13T21:11:31Z

See https://pr-preview.s3.amazonaws.com/rtoy/web-audio-api/pull/2368.html#c2361-1 for an example where we restrict the scope of a change the the smallest reasonable block. This is kind of nice when the changes occur in an algorithm block so we don't include the entire algorithm in the scope of the change block.

I find having the scope of the changed marked out being pretty nice, especially for algorithms since it can be hard to know where the change would end.

rtoy · 2021-07-13T22:25:13Z

Oh, according to https://fantasai.inkedblade.net/style/design/w3c-restyle/2020/readme#proposed-changes, the changes should be marked as proposed corrections/additions.

* The changes should should have the "proposed" class too. * Update text to say "Proposed", as shown in https://fantasai.inkedblade.net/style/design/w3c-restyle/2020/readme#proposed-changes * Update buttons to say "Previous Change" and "Next Change" to make it clearer what the buttons do.

* Limit the scope of each change to a paragraph or item in an algorithm. * Move the changed section to be in the `<div>` for the amendment box to it's easier to see where the changes could be.

rtoy · 2021-07-15T20:27:48Z

Merged in the proposals from #2367 and #2368 into this. Now the boxes include the changed text area and each box covers a smaller change, such as a paragraph or one step in an algorithm.

Also updated the buttons to say "Previous Change" and "Next Change" to make it easier to understand the the previous/next buttons really did.

The id had the correct value, but the link for the issue used the wrong change number (should be 3 instead of 2).

Not needed because we've basically inlined it all so that the preview shows the prev/next buttons and the list of changes for the change log.

hoch · 2021-08-25T22:58:44Z

I think we can merge this.

domenic and others added 2 commits June 25, 2021 15:44

Use new Web IDL buffer primitives

d47c3d7

See whatwg/webidl#987.

This was referenced Jun 28, 2021

Travis CI not working? #2364

Closed

Fix #2359: Fix typo in VUMeterNode #2360

Merged

Raymond Toy added 3 commits July 2, 2021 08:10

Remove link that no longer exists

2b6c92b

Merge branch 'main' into 2361-use-new-idl-buffer-prim

97acb76

Update changelog

7f83179

rtoy commented Jul 2, 2021

View reviewed changes

Raymond Toy added 2 commits July 2, 2021 09:55

Add script to create list of corrections for a change

f029770

Add comments

2acd1fb

rtoy commented Jul 7, 2021

View reviewed changes

Change text slightly

d2303f5

Change "write" to "allow writing into" so reflects better what getChannelData allows.

rtoy mentioned this pull request Jul 8, 2021

RangeError is thrown only for negative cancelTime #2352

Merged

Add links to the previous and next change in the amendment box.

a987c3e

Raymond Toy added 5 commits July 9, 2021 15:29

Inline the JS file

328d71d

Fix up script and add event listener

27d19a9

Fix up the script so bikeshed doesn't mangle it. Add an onload event listener so that when the doc is loaded we can run ListAmendments to add a list of the related changes for the change log.

Fix typo

44ff580

Merge branch '2361-use-new-idl-buffer-prim' into 2361-use-new-idl-buf…

3e81041

…fer-prim-next-buttons

Inline JS into index.bs

59bbd4d

Adjust so bikeshed doesn't mangle it. Move the listener code into the same script segment so it's easy to find.

rtoy mentioned this pull request Jul 12, 2021

2361 use new idl buffer prim next buttons #2366

Closed

rtoy requested review from hoch and padenot July 12, 2021 16:22

Add some comments

a2fb0f4

hoch reviewed Jul 12, 2021

View reviewed changes

Raymond Toy and others added 2 commits July 13, 2021 15:35

Refine amendment boxes

c654440

* Limit the scope of each change to a paragraph or item in an algorithm. * Move the changed section to be in the `<div>` for the amendment box to it's easier to see where the changes could be.

This was referenced Jul 15, 2021

2361 use new idl buffer prim bigger scope #2367

Closed

2361 use new idl buffer prim finer scope #2368

Closed

rtoy added 2 commits July 15, 2021 13:29

Fix bad numbering.

d2285a6

The id had the correct value, but the link for the issue used the wrong change number (should be 3 instead of 2).

Remove amendents.js.

94b46b8

Not needed because we've basically inlined it all so that the preview shows the prev/next buttons and the list of changes for the change log.

padenot approved these changes Aug 12, 2021

View reviewed changes

padenot mentioned this pull request Aug 12, 2021

Use new Web IDL buffer primitives #2358

Closed

hoch approved these changes Aug 25, 2021

View reviewed changes

padenot merged commit 8a75168 into WebAudio:main Aug 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2361: Use new Web IDL buffer primitives #2363

Fix #2361: Use new Web IDL buffer primitives #2363

rtoy commented Jun 28, 2021 •

edited by pr-preview bot

Loading

rtoy commented Jun 29, 2021

rtoy Jul 2, 2021

domenic Jul 2, 2021

domenic Jul 2, 2021

rtoy Jul 7, 2021

rtoy Jul 2, 2021

rtoy Jul 7, 2021

hoch Jul 12, 2021

hoch Jul 12, 2021

rtoy Jul 12, 2021

rtoy commented Jul 9, 2021

rtoy commented Jul 12, 2021

hoch Jul 12, 2021

rtoy Jul 12, 2021

hoch Jul 12, 2021

hoch Jul 12, 2021

hoch Jul 12, 2021

rtoy commented Jul 13, 2021

rtoy commented Jul 13, 2021

rtoy commented Jul 13, 2021

rtoy commented Jul 15, 2021

hoch commented Aug 25, 2021

Fix #2361: Use new Web IDL buffer primitives #2363

Fix #2361: Use new Web IDL buffer primitives #2363

Conversation

rtoy commented Jun 28, 2021 • edited by pr-preview bot Loading

rtoy commented Jun 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtoy commented Jul 9, 2021

rtoy commented Jul 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rtoy commented Jul 13, 2021

rtoy commented Jul 13, 2021

rtoy commented Jul 13, 2021

rtoy commented Jul 15, 2021

hoch commented Aug 25, 2021

rtoy commented Jun 28, 2021 •

edited by pr-preview bot

Loading