Closed oooookk7 closed 7 years ago
See TWP-704 for plugin. Checked with @miniruwan on the actual way of how it should work and now it works.
@miniruwan removed the if else checks for removing mediaSource
flag for plugin.
@johache due to time constraints for urgent release, we are going ahead for the release.
Summary
This PR is to patch Chrome/IE/Safari
"mediaSource"
flag not being honoured, and realised that an entire extension update has to be done. This is for us to ensure our polyfills and extensions are up-to-date as of what's the current state of things in implementations. This PR might have huge code changes.Note that we are not waiting for the extensions to be approved due to time constraints, that will be another mini PR for that.
Changelog:
Added
AdapterJS.extensionInfo
object to allow configuration of custom extension using the AdapterJS extension codebase. Developers can override the extension settings by pre-defining theAdapterJS
object first then loading theAdapterJS
script.Example:
Removed required need to install Firefox addon for version
52
and above.Previously, we require an addon which uses the Bootstrapped API, which allows modification of the browser preferences of
media.getusermedia.screensharing.enabled
to enable screensharing andmedia.getusermedia.screensharing.allowed_domains
) to whitelist the domains.However,
media.getusermedia.screensharing.enabled
has been enabled by default and whitelisting has been removed from52
onwards.Even so, the Bootstrapped API no longer can be used from
53
onwards and uploaded on the addons.mozilla.org (AMO).The extension we have - Skylink WebRTC Tools will work with
51
and below and only with our specified domains.If developers need to enable screensharing for
51
and below, we have to ask them to tell their end-users to modify theabout:config
manually and set themedia.getusermedia.screensharing.enabled
totrue
andmedia.getusermedia.screensharing.allowed_domains
to append the domain they want to enable screensharing.Added checks to ensure that required and correct parameters are provided.
navigator.getUserMedia()
should receive parameters likeconstraints
,successCb
andfailureCb
, else throw anError
.Due to the overrides, when we need to return
failureCb()
for extension disabled case, it will throw anError
with message like "failureCb() is not a method" which instead we could tell the users to provide the correct mandatory parameters first.Browsers generally throw errors if parameters are not satisfied. Try
navigator.mozGetUserMedia({})
ornavigator.webkitGetUserMedia({})
and errors should be thrown.Implemented
try
andcatch
fornavigator.mediaDevices.getUserMedia()
which returns aPromise
to prevent theError
when thrown to be returned inPromise
'sreject
as part of thePromise
methodology.Added stringent checks to ensure the types are expected.
Example, instead of
object && !!object.prop
, do thisobject && typeof object === 'object' && object.hasOwnProperty(prop)
.Older Firefox versions used to give me this when implementing a similar login in the SDK: "Read a property from a non-object but a Boolean" errors before, it's better to add checks with expected types.
Fixes to honour the
"mediaSource"
flag in Chrome / Safari / IE / Firefox.Updated Chrome extensions and detectRTC.html code base to ensure that
"mediaSource"
flag is honoured. The codebase should be able to be backwards compatible.When
"mediaSource"
flag is provided in Safari / IE, accept only values like"screen"
,"window"
or a combination of them in anArray
.AdapterJS.WebRTCPlugin.plugin.screensharingKeys
is not available, fallback to alternative likeAdapterJS.WebRTCPlugin.plugin.screensharingKey || "Screensharing"
.When
"mediaSource"
flag is provided in Chrome / Opera, accept only values like"screen"
,"window"
,"tab"
or a combination of them in anArray
.Additionally accept only types like
["tab", "audio"]
to retrieve tab audio. Ifconstraints.audio
is not enabled (truthy) and["tab", "audio"]
is requested, warn end-users.When
"mediaSource"
flag is provided in Firefox, accept only values like"screen"
,"application"
,"camera"
as described by bugzilla ticket.Additionally accept
"window"
as valid value as described here.If provided as an
Array
, accept the first valid source value. Example where sources are:["tab", "screen", "audio", "window"]
,"screen"
will be selected.For
"browser"
value, the browser preferencemedia.getusermedia.browser.enabled
value has to be set totrue
inabout:config
as described here.Added screensharing support for Android Chrome 59, where
"mediaSource"
should only accept value of"screen"
.chrome://flags
as described in here.Added support for Opera screensharing. This uses the same codebase for the new Chrome extension.
Added flag
AdapterJS._mediaSourcePolyfillIsDefined
to preventAdapterJS._defineMediaSourcePolyfill()
from being invoked again as a form of sanity prevention.AdapterJS.defineMediaSourcePolyfill
toAdapterJS._defineMediaSourcePolyfill
with an added_
prefix as it's an internal method.Testing
Note, we should move these test cases to unit tests.
How is it tested:
adapter.screenshare-old.js
, go to this link.adapter.screenshare-new.js
, checkout to this branch, dogrunt publish
and copy thepublish/adapter.screenshare.js
file.detectRTC-old.html
, go to this link.detectRTC-new.html
, PM.This uses the
navigator.getUserMedia({ audio: true, video: { mediaSource: xx } })
.Chrome
Use the new extension in development mode.
Ensure that when using the new AdapterJS + new DetectRTC + new Chrome extension, whatever
"mediaSource"
is provided is presented as expected. Like"tab"
should result in displaying a list of browser tabs to select. Like["tab", "audio"]
should result in being able to select browser tabs and select audio, or["tab", "audio"]
should return the audio track, or["screen", "window"]
returns a list of screens or windows to select.Ensure that when using any old AdapterJS / DetectRTC / Chrome extension with new AdapterJS / DetectRTC / Chrome extension should not affect previous functionalities where "window" and "screen" sources is presented.
Firefox
Ensure that for Firefox 52 when PermissionDenied, the popup installation doesn't trigger.
Ensure that for Firefox 51 and below when PermissionDenied, the popup installation does trigger.
Ensure that "camera", "browser", "application" are valid sources for
"mediaSource"
.Opera
Use the new extension in development mode.
Test cases tested:
Chrome extension test:
Extension tested in development mode.
0.1.4
0.1.2
0.1.1
0.1.3
0.1.1
0.1.4
0.1.2
0.1.3
0.1.2
0.1.4
0.14.1
0.1.1
0.1.3
0.14.1
0.1.1
0.1.4
0.14.1
0.1.2
0.1.3
0.14.1
0.1.2
0.1.4
Firefox versions test:
52
49
Constraints test:
Extensions are tested in development mode, except for Firefox. Note that for the current Chrome extension
ljckddiekopnnjoeaiofddfhgnbdoafc
as a fallback alternative will not honour the"mediaSource"
flag and return["window", "screen"]
as expected. This should be different in the new Chrome extension when approved."mediaSource"
"tab"
"window"
"screen"
["tab", "audio"]
audio:false
["tab", "audio"]
audio:true
["tab", "screen"]
["tab", "window"]
["window", "screen"]
["window", "screen", "tab"]
["screen"]
"screen"
["screen"]
"screen"
["window"]
"window"
["camera"]
"camera"
["browser"]
"browser"
["application"]
"application"
"screen"
"window"
["screen", "window"]