Issue #83: Move media-source spec to Contiguous IDL #127

mwatson2 · 2016-07-20T01:31:09Z

No description provided.

wolenetz · 2016-07-20T22:50:51Z

@mwatson2 : media-source.html should not be changed by this PR. It needs to remain as a redirector to index.html. Thanks for preparing this. I'll do further review on this before EOW.

jdsmith3000 · 2016-07-21T18:51:53Z

The new IDL text seems to render fine in HTML, but isn't very readable in text form because it's one long run-on line. I've prefer these be broken up, like the examples in the Contiguous IDL document:

  [Constructor]
  interface Dahut : Mammal {
    const unsigned short LEVROGYROUS = 0;
    const unsigned short DEXTROGYROUS = 1;
    readonly attribute DOMString chirality;
    attribute unsigned long age;
    Dahut turnAround(float angle, boolean fall);
    unsigned long trip();
    void yell([AllowAny] unsigned long volume,
              [TreatNullAs=EmptyString] DOMString sentence);
  };

You mentioned the conversion was largely done by an updated version of respec. Does it do the current formatting?

mwatson2 · 2016-07-21T19:29:25Z

media-source.html should not be changed by this PR

Oops. Fixed that.

The new IDL text seems to render fine in HTML, but isn't very readable in text form because it's one long run-on line.

Not sure what you mean. Do you mean there is a case there is IDL is on one run-on line ? Or are you talking about the HTML for the attribute / method tables ?

You mentioned the conversion was largely done by an updated version of respec. Does it do the current formatting?

Yes, it does what ReSpec does with "Old School" IDL (convert to the HTML for the IDL itself, the method and attribute tables and description sections) and nothing else. So, what you are left with is Contiguous IDL. Then you need to go in and patch up all the links, which have changed.

jdsmith3000 · 2016-07-21T19:39:53Z

I did an update and don't see as clear of an example as previously. The formatting is ragged around the IDL content though, like this:

       <div><pre class="idl">partial interface Navigator {
    [SecureContext] Promise&lt;MediaKeySystemAccess&gt; requestMediaKeySystemAccess (DOMString keySystem, sequence&lt;MediaKeySystemConfiguration&gt; supportedConfigurations);
};</pre><section><h2>Methods</h2><dl class="methods" data-dfn-for="Navigator" data-link-for="Navigator"><dt><dfn><code>requestMediaKeySystemAccess</code></dfn></dt><dd>

mwatson2 · 2016-07-21T19:46:29Z

@jdsmith3000 Yes, it didn't format the HTML it generated nicely. We could tidy it up by hand (in one go, or as we work on each part of the spec) or we could use a tool to reformat the whole thing.

jdsmith3000 · 2016-07-22T21:53:47Z

If you know of a tool, that would be great. If not, the cleanup looks straightforward and wouldn't be too hard to do manually. We could merge this change (since it is functional) and enter an issue for cleanup, if there is time.

wolenetz · 2016-07-26T22:12:21Z

media-source-respec.html

+interface MediaSource : EventTarget {
+    readonly        attribute SourceBufferList    sourceBuffers;
+    readonly        attribute SourceBufferList    activeSourceBuffers;
+    readonly        attribute ReadyState          readyState;


index.html no longer contains a hyperlink from "readyState" to the attribute's definition from the WebIDL table. Please fix.

This is a ReSpec bug: w3c/respec#893

Does the respec bug also encompass the enum link I just noted for enum ReadyState?

Yes, I expect so. It gets confused internally between readyState and ReadyState.

jdsmith3000 · 2016-07-26T22:40:07Z

@wolenetz I'm glad you caught these. Note sure how I missed them.

@mwatson2 Are these the type of links that respec doesn't migrate to contiguous IDL properly?

wolenetz · 2016-07-26T22:52:48Z

media-source-respec.html

-      <dl title="interface SourceBufferList : EventTarget" class="idl">
-        <dt>readonly attribute unsigned long length</dt>
-        <dd>
+      <div><pre class="idl">interface SourceBufferList : EventTarget {


Ditto (many types missing links here, in this object's IDL, attribute and method details...)

wolenetz · 2016-07-26T23:08:03Z

@mwatson2 Please also update the publishDate (or I can do that if that's all that's remaining before merging).
I just finished this iteration of reviewing this. Other than publishDate, I think the main issue is many of the types in the IDL, attribute and method detail no longer have links when they did before this change.

wolenetz · 2016-07-26T23:14:41Z

Note that when I was originally trying to fix those respec errors, I hit a similar problem where the links evaporated: see #68 (comment)

wolenetz · 2016-07-28T22:33:18Z

I've finished this iteration of review as of 1f5f9f3 -- it looks much better, @mwatson2. Thank you! If w3c/respec#893 only impacts readyState and appendBuffer, it seems we could file another V1Editorial to fix-up those (or work-around those) separately.
Current outstanding review comments:

mwatson2 · 2016-07-28T23:56:49Z

@wolenetz In response to the several "Is this expected ?", the conversion script is very ad hoc and such instructions as I was given note that you'll have to fix up some links, so in that sense, yes, it is expected that it will break some links and mess up some formatting which we will then have to fix.

Regarding w3c/respec#893, I only just filed it so I'm not sure there's an ETA. If I knew how to work around it, I'd just fix the underlying bug myself ;-)

The missing appendBuffer links is likely a different ReSpec issue: there is nothing we can do in the source to fix it, since the source is just the pure IDL. I did ask about this issue and it was suggested to replace the two methods with:

void appendBuffer( (ArrayBuffer or ArrayBufferView) data );

(WebIDL apparently doesn't really have overloading, but it does have union types).

So, I will fix the formatting issues.

wolenetz · 2016-07-29T17:04:21Z

Sounds good. If we need to work around the respec bug manually in
index.html please document the steps so that we can publish a clean
version. Thanks for working on this, Mark.

On Jul 28, 2016 4:56 PM, "mwatson2" notifications@github.com wrote:

@wolenetz https://github.com/wolenetz In response to the several "Is
this expected ?", the conversion script is very ad hoc and such
instructions as I was given note that you'll have to fix up some links, so
in that sense, yes, it is expected that it will break some links and mess
up some formatting which we will then have to fix.

Regarding w3c/respec#893 w3c/respec#893, I
only just filed it so I'm not sure there's an ETA. If I knew how to work
around it, I'd just fix the underlying bug myself ;-)

The missing appendBuffer links is likely a different ReSpec issue: there
is nothing we can do in the source to fix it, since the source is just the
pure IDL. I did ask about this issue and it was suggested to replace the
two methods with:

void appendBuffer( (ArrayBuffer or ArrayBufferView) data );

(WebIDL apparently doesn't really have overloading, but it does have union
types).

So, I will fix the formatting issues.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#127 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ALCtwwX_rz6NL_cHdyh13NMfuvR_gXi1ks5qaUHDgaJpZM4JQVpO
.

mwatson2 · 2016-07-29T20:53:00Z

I have a question in on the ReSpec bug. A cannot find a way to fix the appendBuffer problem. We might have to patch that up by hand in media-source.js.

The other problems should be fixed now.

jdsmith3000 · 2016-07-29T23:39:46Z

media-source-respec.html

-      <dl title="enum ReadyState" class="idl">
-        <dt>closed</dt>
-        <dd>
+      <div><pre class="idl">enum ReadyState {


The enum ReadyState is linked to the readyState attribute description, and that attribute (lower down) has no link.

jdsmith3000 · 2016-07-29T23:58:12Z

It looks like both the ReadyState enum and attribute have link problems still as well.

I also noted a link from the TrackDefaultList interface to it's constructor that is new.

dontcallmedom · 2016-08-01T16:21:17Z

Regarding the problem of auto-linking of readyState, I think it can be solved without patching ReSpec: replacing the <dt> where readyState is defined with the following seems to do the trick:
<dt><dfn data-dfn-for='MediaSource' data-lt='readyState'><code>readyState</code></dfn> of type <span class="idlAttrType"><a href="#idl-def-readystate">ReadyState</a></span>, readonly </dt>

With that, the id dom-readystate disappears, which breaks a number of internal links based on def-id which seems a local thing.

mwatson2 · 2016-08-02T18:27:47Z

This PR now includes the work-around for the readyState link problem as well as 6 other issues (per the commits above). I think it is now good-to-go.

mwatson2 · 2016-08-03T01:20:10Z

I believe all the issues are fixed now. Shall I merge this ?

jdsmith3000 · 2016-08-03T03:06:31Z

The only odd thing I noticed was the ReadyState enum has a link to itself, where the common format would have no link. Right?

wolenetz · 2016-08-08T21:04:34Z

I've landed some large chunks (removing the 3 at-risk features) on gh-pages, so before reviewing, I'll try to add a merge commit to this PR that resolves those conflicts.

wolenetz · 2016-08-08T23:17:38Z

Looks mostly good. There's still a little weirdness around "ReadyState" type of the "readyState" attribute linking to the readyState attribute IDL, not the ReadyState enum. I believe that's the only issue I spotted related to this PR when I merged it with current gh-pages locally. I'll attempt to add that merge commit to this PR now.

wolenetz · 2016-08-08T23:23:13Z

As I expected, I don't have permissions to push to mwatson2's fork of w3c/media-source. I'll redo this PR using my own fork and post that PR reference here shortly.

wolenetz · 2016-08-08T23:27:30Z

See PR #141 for a more current version of this PR. Closing this one since it is replaced by #141.

Issue w3c#83: Move media-source spec to Contiguous IDL

6b7b95c

mwatson2 mentioned this pull request Jul 20, 2016

Fix respec errors and retain wedIDLv1 hyperlinks #83

Closed

mwatson2 added 2 commits July 21, 2016 12:16

Restore correct version of media-source.html

a674d36

Merge branch 'gh-pages' into issue-83

6284251

wolenetz reviewed Jul 26, 2016
View reviewed changes

mwatson2 added 4 commits July 29, 2016 11:06

Missing js file

815e93d

Restore links for parameter types in parameter tables

31bf934

Add external links to return types

91491c8

Restore parameter descriptions to tables where missing

0b706cc

jdsmith3000 reviewed Jul 29, 2016
View reviewed changes

mwatson2 added 8 commits August 2, 2016 08:38

Work around ReSpec#893 for (R|r)eadyState

ac5624e

Use WebIDL BufferSource instread of appendBuffer overload

8e1458d

Fix SourceBuffer links

2ad6a2a

Fix links for TrackDefault and TrackDefaultList

06204de

Fix duplicate definiton error for getter

7dec53b

Restore simple name for constructors

7a830b9

Restore Constructor, Attribute, Method sections to ToC

b71ef43

Merge branch 'gh-pages' into issue-83

255fcbe

ddorwin mentioned this pull request Aug 8, 2016

WebIDL has link errors after contiguous IDL conversion w3c/encrypted-media#293

Open

wolenetz mentioned this pull request Aug 8, 2016

Wolenetz adding to mwatson2 issue 83 in wolenetz repo #141

Merged

wolenetz closed this Aug 8, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #83: Move media-source spec to Contiguous IDL #127

Issue #83: Move media-source spec to Contiguous IDL #127

mwatson2 commented Jul 20, 2016

wolenetz commented Jul 20, 2016

jdsmith3000 commented Jul 21, 2016

mwatson2 commented Jul 21, 2016

jdsmith3000 commented Jul 21, 2016

mwatson2 commented Jul 21, 2016

jdsmith3000 commented Jul 22, 2016

wolenetz Jul 26, 2016

mwatson2 Jul 28, 2016

jdsmith3000 Jul 29, 2016

mwatson2 Jul 30, 2016

jdsmith3000 commented Jul 26, 2016

wolenetz Jul 26, 2016 •

edited

wolenetz commented Jul 26, 2016

wolenetz commented Jul 26, 2016

wolenetz commented Jul 28, 2016

mwatson2 commented Jul 28, 2016

wolenetz commented Jul 29, 2016

mwatson2 commented Jul 29, 2016

jdsmith3000 Jul 29, 2016

jdsmith3000 commented Jul 29, 2016

dontcallmedom commented Aug 1, 2016

mwatson2 commented Aug 2, 2016

mwatson2 commented Aug 3, 2016

jdsmith3000 commented Aug 3, 2016 •

edited

wolenetz commented Aug 8, 2016

wolenetz commented Aug 8, 2016

wolenetz commented Aug 8, 2016

wolenetz commented Aug 8, 2016

Issue #83: Move media-source spec to Contiguous IDL #127

Issue #83: Move media-source spec to Contiguous IDL #127

Conversation

mwatson2 commented Jul 20, 2016

wolenetz commented Jul 20, 2016

jdsmith3000 commented Jul 21, 2016

mwatson2 commented Jul 21, 2016

jdsmith3000 commented Jul 21, 2016

mwatson2 commented Jul 21, 2016

jdsmith3000 commented Jul 22, 2016

wolenetz Jul 26, 2016

Choose a reason for hiding this comment

mwatson2 Jul 28, 2016

Choose a reason for hiding this comment

jdsmith3000 Jul 29, 2016

Choose a reason for hiding this comment

mwatson2 Jul 30, 2016

Choose a reason for hiding this comment

jdsmith3000 commented Jul 26, 2016

wolenetz Jul 26, 2016 • edited

Choose a reason for hiding this comment

wolenetz commented Jul 26, 2016

wolenetz commented Jul 26, 2016

wolenetz commented Jul 28, 2016

mwatson2 commented Jul 28, 2016

wolenetz commented Jul 29, 2016

mwatson2 commented Jul 29, 2016

jdsmith3000 Jul 29, 2016

Choose a reason for hiding this comment

jdsmith3000 commented Jul 29, 2016

dontcallmedom commented Aug 1, 2016

mwatson2 commented Aug 2, 2016

mwatson2 commented Aug 3, 2016

jdsmith3000 commented Aug 3, 2016 • edited

wolenetz commented Aug 8, 2016

wolenetz commented Aug 8, 2016

wolenetz commented Aug 8, 2016

wolenetz commented Aug 8, 2016

wolenetz Jul 26, 2016 •

edited

jdsmith3000 commented Aug 3, 2016 •

edited