Issue 4720 - Edge packager does not support devenv

Issue 4720 - Edge packager does not support devenv

[kzar, 2016-12-12 12:08:36]

LGTM

[Sebastian Noack, 2016-12-13 14:37:34]

Note that chromeDevenvPoller__.js which is included when using devenv is using
chrome.runtime.reload(). Does that API even exist on Edge?

[Oleksandr, 2016-12-13 16:29:33]

On 2016/12/13 14:37:34, Sebastian Noack wrote:
> Note that chromeDevenvPoller__.js which is included when using devenv is using
> chrome.runtime.reload(). Does that API even exist on Edge?

chrome.runtime.reload() does not exist in Edge, however it is never invoked
there. We add devenvpoller.js to backgroundscripts, but we don't actually
include it in the package. So Edge just silently fails to load it.

I have also rebased the patchset to the current tip.

[Oleksandr, 2016-12-13 16:29:33]

On 2016/12/13 14:37:34, Sebastian Noack wrote:
> Note that chromeDevenvPoller__.js which is included when using devenv is using
> chrome.runtime.reload(). Does that API even exist on Edge?

chrome.runtime.reload() does not exist in Edge, however it is never invoked
there. We add devenvpoller.js to backgroundscripts, but we don't actually
include it in the package. So Edge just silently fails to load it.

I have also rebased the patchset to the current tip.

[Sebastian Noack, 2016-12-14 12:23:46]

On 2016/12/13 16:29:33, Oleksandr wrote:
> chrome.runtime.reload() does not exist in Edge, however it is never invoked
> there. We add devenvpoller.js to backgroundscripts, but we don't actually
> include it in the package. So Edge just silently fails to load it.

Perhaps we should include it, and make it detect whether runtime.reload() exist,
so that it will work on Edge as soon as they add that API.

[Sebastian Noack, 2017-09-23 18:25:22]

On 2016/12/14 12:23:46, Sebastian Noack wrote:
> On 2016/12/13 16:29:33, Oleksandr wrote:
> > chrome.runtime.reload() does not exist in Edge, however it is never invoked
> > there. We add devenvpoller.js to backgroundscripts, but we don't actually
> > include it in the package. So Edge just silently fails to load it.
> 
> Perhaps we should include it, and make it detect whether runtime.reload()
exist,
> so that it will work on Edge as soon as they add that API.

It seems Microsoft Edge supports runtime.reload() now:
https://docs.microsoft.com/en-us/microsoft-edge/extensions/api-support/suppor...

Can you confirm? If so, all we'd need to do, it seems, is rebasing this patch,
can you do this as well? Thanks.

[Oleksandr, 2017-09-25 08:26:24]

runtime.reload() is supported now indeed. Rebased.

[Sebastian Noack, 2017-09-25 15:58:53]

https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py
File packagerEdge.py (right):

https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py#new...
packagerEdge.py:191: if devenv:
This logic is duplicated in packagerChrome.py. Can you moveit into a function
and reuse that function here?

Also is it intentional that you left out the qunit test suite?

[Oleksandr, 2017-10-13 14:33:47]

On 2017/09/25 15:58:53, Sebastian Noack wrote:
> https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py
> File packagerEdge.py (right):
> 
>
https://codereview.adblockplus.org/29367148/diff/29555628/packagerEdge.py#new...
> packagerEdge.py:191: if devenv:
> This logic is duplicated in packagerChrome.py. Can you moveit into a function
> and reuse that function here?
> 
> Also is it intentional that you left out the qunit test suite?

No, it was unintentional that I left out qunit test suite. Since Tristan is
going to take this over, and basically the review would look different after
reusing the code in packagerChrome.py I will close this codereview in favor of
whatever Tristan will submit.