Add compat data for RTCPeerConnection by bunnybooboo · Pull Request #1070 · mdn/browser-compat-data

bunnybooboo · 2018-02-13T22:38:39Z

Via #987
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection

jpmedley · 2018-02-13T23:17:42Z

api/RTCPeerConnection.json

+            }
+          ],
+          "chrome": {
+            "version_added": true


This should be the same as chrome_android and webview_android.

jpmedley · 2018-02-13T23:18:17Z

api/RTCPeerConnection.json

+            "version_added": null
+          },
+          "opera_android": {
+            "version_added": null


Both versions of Opera are 43 for unprefixed. https://www.chromestatus.com/features/5312661013135360

jpmedley · 2018-02-13T23:18:49Z

api/RTCPeerConnection.json

+              "version_added": "56"
+            },
+            "chrome": {
+              "version_added": true


jpmedley · 2018-02-13T23:19:16Z

api/RTCPeerConnection.json

+              "version_added": null
+            },
+            "opera_android": {
+              "version_added": null


43 for both Opera

jpmedley · 2018-02-13T23:19:37Z

api/RTCPeerConnection.json

+              "version_added": "56"
+            },
+            "chrome": {
+              "version_added": true


jpmedley · 2018-02-13T23:31:12Z

api/RTCPeerConnection.json

+            "opera": {
+              "version_added": null
+            },
+            "opera_android": {


jpmedley · 2018-02-13T23:31:17Z

api/RTCPeerConnection.json

+              "version_added": "56"
+            },
+            "chrome": {
+              "version_added": true


jpmedley · 2018-02-13T23:31:26Z

api/RTCPeerConnection.json

+              "version_added": null
+            },
+            "opera_android": {
+              "version_added": null


jpmedley · 2018-02-13T23:31:35Z

api/RTCPeerConnection.json

+              "version_added": "56"
+            },
+            "chrome": {
+              "version_added": true


jpmedley · 2018-02-13T23:31:41Z

api/RTCPeerConnection.json

+              "version_added": null
+            },
+            "opera_android": {
+              "version_added": null


jpmedley · 2018-02-15T02:01:06Z

api/RTCPeerConnection.json

+                "notes": "Promise based version."
+              },
+              {
+                "version_added": "56"


The numbers are backwards. Also, this information applies to Webview and Android.

jpmedley · 2018-02-15T02:01:48Z

api/RTCPeerConnection.json

+              },
+              {
+                "version_added": "43"
+              }


The version numbers for both Operas is backwards.

jpmedley · 2018-02-15T02:14:10Z

api/RTCPeerConnection.json

+              },
+              {
+                "version_added": "56"
+              }


Am I giving you bad instructions? Added in 50. Promise-based version in 56. All 3 versions of Chrome.

I was up until 2am fixing this file. The final commit 34cae58 is only 2537 lines long. The review you did was unfortunately on one of the 2 previously failing commits in Travis. It was late, errors were being made, and I believe I finally fixed them. The line numbers you quote there are out of scope of the final commit.

When looking at your review vs what is in the file, they match up.

The link you shared discussing M50..

Add promise-based versions of RTCPeerConnection methods: setLocalDescription, setRemoteDescription, addIceCandidate, createOffer and createAnswer. To be done in 2 steps. First, setLocalDescription, setRemoteDescription and addIceCandidate (anticipated in M50).

Under the light of a new day this now does not look quite right, as it suggests the removal of promises in 56. Seeking out another potential code example inside BCD to replicate, to avoid unecessary repetitious notes in a fix.

Adding the line "notes": "Promise based version and unprefixed." to an immediate commit. This will communicate promises did not STOP in 56, in all instances of earlier rolled out subfeatures. Reflected in f0f62a8

jpmedley · 2018-02-15T02:16:29Z

api/RTCPeerConnection.json

+              "version_added": "43"
+            },
+            "opera_android": {
+              "version_added": "43"


Added in 37. Promise-based in 43. Both versions of Opera.

Actioning this right now.

jpmedley · 2018-02-15T02:18:03Z

api/RTCPeerConnection.json

+              {
+                "version_added": "56"
+              }
+            ],


The numbers are backwards, all three versions of Chrome. Please check this throughout.

jpmedley · 2018-02-15T02:19:06Z

api/RTCPeerConnection.json

+              "version_added": "43"
+            },
+            "opera_android": {
+              "version_added": "43"


Add in 37. Promise-based in 43. Both versions of Opera. Please check this throughout.

bunnybooboo · 2018-02-15T16:04:52Z

Wahoo! Thanks for your patience @jpmedley. I had a steep learning curve in the last week. This file was a massive test on me and you truly helped me get to grips with it. Apologies if it was unusually labor intensive.

jpmedley · 2018-02-15T16:40:09Z

No. Thank you. That's one less that I have to correct. (And I have a bunch.)

Elchi3

Thanks @bunnybooboo! This is a massive amount of work. Well done! Sorry for being slow with reviews, this repo is quite busy.

I have two comments that apply to the whole file. See below

There are 5 sub features that I couldn't find entries for, but I would be OK with adding this as a separate PR as this is gigantic already.
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/close
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/addTransceiver
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/currentRemoteDescription
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/iceGatheringState
https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/onicecandidateerror

Elchi3 · 2018-02-26T15:09:48Z

api/RTCPeerConnection.json

+              "version_removed": "56"
+            },
+            {
+              "notes": "unprefixed",


We don't mark unprefixed separately. So this needs to be a block like this, with the unprefixed / "normal" support first:

"webview_android": [ { "version_added": "56" }, { "prefix": "webkit", "version_added": "51", "version_removed": "56" } ],

This is an issue throughout the file. I'm not marking it everywhere. Please double-check this.

Elchi3 · 2018-02-26T15:13:09Z

api/RTCPeerConnection.json

+                "version_removed": "43"
+              },
+              {
+                "version_added": "43",


Here as well. The most relevant version needs to come first. Swap this around please:

{ "version_added": "43", "notes": "Promise based version." }, { "version_added": "37", "version_removed": "43" }

I'm on it..

…mpat-data into RTCPeerConnection

ExE-Boss · 2018-03-04T01:37:22Z

api/RTCPeerConnection.json

+          },
+          "firefox_android": [
+            {
+              "notes": "unprefixed",


This line is unnecessary, because there is no prefix value defined.

ahh that one slipped through the net.. I'm on it

Elchi3

Thanks for your fixes! Lets get this in. I will file a follow-up issue for the missing sub features and we can do correction PRs if we want to fix more stuff here. This way the massiveness about this should be gone going forward hopefully. Thanks again for this PR, epic amount of work!

This PR corrects the data throughout the RTCPeerConnection API, which was a **big** mess, mostly due to copy-paste error. The fixes include the following: - Converts the "Promise-based version" notes into subfeatures. It is more standard to use a subfeature rather than the notes. Additionally, they were copied and pasted all across the Opera data, so it caused some major data discrepancies. (Particularly, claiming support for features in Opera that weren't supported, wrong version numbers...) Fixes #11158. - Corrects the data regarding promise-based versions for Chrome. After further digging in, it turns out that most of these features had the wrong version number. New data comes from the commit history ([1](https://source.chromium.org/chromium/chromium/src/+/e16c96f5a5b517a64fdbc5be75f05e359e04b461), [2](https://source.chromium.org/chromium/chromium/src/+/70302a3d99ed149d59acf592edf9d32d4fd0dbda), [3](https://source.chromium.org/chromium/chromium/src/+/68f8e256dc7c7369127bd345100bcf776fa8acc0)). - Corrects the Samsung Internet and Opera/Opera Android data. In the case of Samsung Internet, it appears to have been set to 6.0 most everywhere after a mirroring from the old, incorrect wiki data (#1606), and Opera's data initially came from the wiki tables (#1070). When the Chrome data was corrected, however (see #3287), only the Chrome, Chrome Android, and WebView data was adjusted, leaving these other browsers untouched. - Corrects the version number for `createDataChannel` based upon results from the mdn-bcd-collector project (v3.3.0). - Finally, mirrors the Chrome data to both Chrome Android and WebView. Both had been left as `true` for many of the features, however since we have ranged values, we're able to replace the `true` with `≤37` for WebView, and mirror Chrome straight to Chrome Android since we know Chrome Android has supported this API back then.

Add compat data for RTCPeerConnection

a6287b6

jpmedley suggested changes Feb 13, 2018

View reviewed changes

Elchi3 added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Feb 14, 2018

One third through file. Edit Chrome & Opera throughout.

d4bc206

bunnybooboo force-pushed the RTCPeerConnection branch from f3ce8e6 to d4bc206 Compare February 14, 2018 18:52

bunnybooboo added 4 commits February 14, 2018 19:58

Add compat data for RTCPeerConnection

27045d3

One third through file. Edit Chrome & Opera throughout.

b8fa348

Edited chrome & opera throughout

73fcdbb

Add modules and correct via review feedback

34da639

bunnybooboo force-pushed the RTCPeerConnection branch from d4bc206 to 34da639 Compare February 15, 2018 00:01

bunnybooboo added 2 commits February 15, 2018 01:03

Add version_removed to promise based versions of chrome

d71559a

Fix of a number of errors

34cae58

jpmedley suggested changes Feb 15, 2018

View reviewed changes

bunnybooboo added 2 commits February 15, 2018 09:23

Add notes on promise AND unprefixed

f0f62a8

Add Opera notes: added in 37, promises aded in 43

f5d54cb

jpmedley approved these changes Feb 15, 2018

View reviewed changes

Elchi3 requested changes Feb 26, 2018

View reviewed changes

bunnybooboo added 3 commits March 3, 2018 21:21

Add Opera notes: added in 37, promises aded in 43

4155ee3

Merge branch 'RTCPeerConnection' of github.com:bunnybooboo/browser-co…

52f2a0c

…mpat-data into RTCPeerConnection

Editing Chrome and Opera via code review

887160c

ExE-Boss suggested changes Mar 4, 2018

View reviewed changes

Remove prefix line 48

fe7cbc7

Elchi3 approved these changes Mar 6, 2018

View reviewed changes

Elchi3 merged commit 6c81a4b into mdn:master Mar 6, 2018

Elchi3 mentioned this pull request Mar 6, 2018

Update and complete RTCPeerConnection #1288

Closed

bunnybooboo deleted the RTCPeerConnection branch March 7, 2018 18:52

dontcallmedom pushed a commit to dontcallmedom/browser-compat-data that referenced this pull request Mar 17, 2018

Add compat data for RTCPeerConnection (mdn#1070)

53ae181

mlbrgl pushed a commit to mlbrgl/browser-compat-data that referenced this pull request Mar 29, 2018

Add compat data for RTCPeerConnection (mdn#1070)

5dab4aa

queengooborg mentioned this pull request Sep 8, 2021

Correct data for the RTCPeerConnection API #12300

Merged

Conversation

bunnybooboo commented Feb 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bunnybooboo Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bunnybooboo Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bunnybooboo commented Feb 15, 2018

Uh oh!

jpmedley commented Feb 15, 2018

Uh oh!

Elchi3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ExE-Boss Mar 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Elchi3 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

bunnybooboo Feb 15, 2018 •

edited

Loading

bunnybooboo Feb 15, 2018 •

edited

Loading

Elchi3 left a comment •

edited

Loading

ExE-Boss Mar 4, 2018 •

edited

Loading