You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/10/21 07:35:13 UTC
[GitHub] [cordova-serve] RedMickey opened a new pull request #40: Add MS Edge Chromium support
RedMickey opened a new pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40
Hello,
I am a maintainer of [Cordova Tools](https://github.com/microsoft/vscode-cordova) VS Code extension and [Cordova-Simulate](https://github.com/microsoft/cordova-simulate) tool. We use the `cordova-serve` package in `Cordova-Simulate` for simulating Apache Cordova applications on different platforms (Android, iOS) in the browser.
### Platforms affected
- Microsoft Edge based on Chromium
### Motivation and Context
We have an [issue](https://github.com/microsoft/vscode-cordova/issues/588) about adding support of MS Edge Chromium. Since in `Cordova-Simulate` we use the `cordova-serve` package for launching apps in the browser, we consider that it makes sense to add a support of Edge Chromium to `cordova-serve` instead of making a local enhancement on `Cordova-Simulate` side. Also we think that it would be a useful enhancement for the `cordova-serve` package which allows to use the package for serving Cordova apps in MS Edge Chromium.
### Description
Since the old Edge browser had become deprecated we added support of launching MS Edge Chromium on Windows and macOS.
This is a breaking change, since we removed support of non Chromium MS Edge browser. As seen Microsoft is promoting MS Edge Chromium browser and trying to replace the previous version of Edge which is not based on Chromium. We think that sooner or later the old Edge will be completely replaced on all the systems, so this PR could prepare the package to that change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] timbru31 commented on a change in pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#discussion_r756497745
##########
File path: src/browser.js
##########
@@ -32,7 +32,7 @@ const NOT_SUPPORTED = 'The browser target is not supported: %target%';
* Launches the specified browser with the given URL.
* Based on https://github.com/domenic/opener
* @param {{target: ?string, url: ?string, dataDir: ?string}} opts - parameters:
- * target - the target browser - ie, chrome, safari, opera, firefox or chromium
+ * target - the target browser - ie, edge, chrome, safari, opera, firefox or chromium
Review comment:
Whoops
##########
File path: src/browser.js
##########
@@ -32,7 +32,7 @@ const NOT_SUPPORTED = 'The browser target is not supported: %target%';
* Launches the specified browser with the given URL.
* Based on https://github.com/domenic/opener
* @param {{target: ?string, url: ?string, dataDir: ?string}} opts - parameters:
- * target - the target browser - ie, chrome, safari, opera, firefox or chromium
+ * target - the target browser - ie, edge, chrome, safari, opera, firefox or chromium
Review comment:
```suggestion
* target - the target browser - ie, edge ,edgechromium, chrome, safari, opera, firefox or chromium
```
For the sake of completeness
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] codecov-io commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-730219268
# [Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=h1) Report
> Merging [#40](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=desc) (56b80e2) into [master](https://codecov.io/gh/apache/cordova-serve/commit/ea89b6ee6d0c84a018c7b893bfbfead97b1c402b?el=desc) (ea89b6e) will **increase** coverage by `2.69%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #40 +/- ##
==========================================
+ Coverage 28.11% 30.80% +2.69%
==========================================
Files 6 6
Lines 217 198 -19
==========================================
Hits 61 61
+ Misses 156 137 -19
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/browser.js](https://codecov.io/gh/apache/cordova-serve/pull/40/diff?src=pr&el=tree#diff-c3JjL2Jyb3dzZXIuanM=) | `23.88% <0.00%> (+5.27%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=footer). Last update [ea89b6e...046230d](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] codecov-io edited a comment on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-730219268
# [Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=h1) Report
> Merging [#40](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=desc) (4d41b6d) into [master](https://codecov.io/gh/apache/cordova-serve/commit/ea89b6ee6d0c84a018c7b893bfbfead97b1c402b?el=desc) (ea89b6e) will **not change** coverage.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #40 +/- ##
=======================================
Coverage 28.11% 28.11%
=======================================
Files 6 6
Lines 217 217
=======================================
Hits 61 61
Misses 156 156
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/browser.js](https://codecov.io/gh/apache/cordova-serve/pull/40/diff?src=pr&el=tree#diff-c3JjL2Jyb3dzZXIuanM=) | `18.60% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=footer). Last update [ea89b6e...4d41b6d](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] erisu commented on a change in pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
erisu commented on a change in pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#discussion_r756489444
##########
File path: src/browser.js
##########
@@ -32,7 +32,7 @@ const NOT_SUPPORTED = 'The browser target is not supported: %target%';
* Launches the specified browser with the given URL.
* Based on https://github.com/domenic/opener
* @param {{target: ?string, url: ?string, dataDir: ?string}} opts - parameters:
- * target - the target browser - ie, chrome, safari, opera, firefox or chromium
+ * target - the target browser - ie, edge, chrome, safari, opera, firefox or chromium
Review comment:
I think `ie` in this case was refering to "Internet Explorer" and not "In Example"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] SounD120 commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
SounD120 commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-748053112
Hi @erisu , @timbru31 . Could you please take a look on this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] codecov-io edited a comment on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-730219268
# [Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=h1) Report
> Merging [#40](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=desc) (046230d) into [master](https://codecov.io/gh/apache/cordova-serve/commit/ea89b6ee6d0c84a018c7b893bfbfead97b1c402b?el=desc) (ea89b6e) will **increase** coverage by `2.69%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #40 +/- ##
==========================================
+ Coverage 28.11% 30.80% +2.69%
==========================================
Files 6 6
Lines 217 198 -19
==========================================
Hits 61 61
+ Misses 156 137 -19
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/browser.js](https://codecov.io/gh/apache/cordova-serve/pull/40/diff?src=pr&el=tree#diff-c3JjL2Jyb3dzZXIuanM=) | `23.88% <0.00%> (+5.27%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=footer). Last update [ea89b6e...046230d](https://codecov.io/gh/apache/cordova-serve/pull/40?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] RedMickey commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
RedMickey commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-730224761
Hi @erisu, could you please review this PR?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] timbru31 commented on a change in pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
timbru31 commented on a change in pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#discussion_r756483922
##########
File path: src/browser.js
##########
@@ -32,7 +32,7 @@ const NOT_SUPPORTED = 'The browser target is not supported: %target%';
* Launches the specified browser with the given URL.
* Based on https://github.com/domenic/opener
* @param {{target: ?string, url: ?string, dataDir: ?string}} opts - parameters:
- * target - the target browser - ie, chrome, safari, opera, firefox or chromium
+ * target - the target browser - ie, edge, chrome, safari, opera, firefox or chromium
Review comment:
```suggestion
* target - the target browser - ie, edge,edgechromium, chrome, safari, opera, firefox or chromium
```
For the sake of completeness
##########
File path: src/browser.js
##########
@@ -32,7 +32,7 @@ const NOT_SUPPORTED = 'The browser target is not supported: %target%';
* Launches the specified browser with the given URL.
* Based on https://github.com/domenic/opener
* @param {{target: ?string, url: ?string, dataDir: ?string}} opts - parameters:
- * target - the target browser - ie, chrome, safari, opera, firefox or chromium
+ * target - the target browser - ie, edge, chrome, safari, opera, firefox or chromium
Review comment:
```suggestion
* target - the target browser - ie, edge ,edgechromium, chrome, safari, opera, firefox or chromium
```
For the sake of completeness
##########
File path: src/browser.js
##########
@@ -32,7 +32,7 @@ const NOT_SUPPORTED = 'The browser target is not supported: %target%';
* Launches the specified browser with the given URL.
* Based on https://github.com/domenic/opener
* @param {{target: ?string, url: ?string, dataDir: ?string}} opts - parameters:
- * target - the target browser - ie, chrome, safari, opera, firefox or chromium
+ * target - the target browser - ie, edge, chrome, safari, opera, firefox or chromium
Review comment:
```suggestion
* target - the target browser - i.e., edge ,edgechromium, chrome, safari, opera, firefox or chromium
```
For the sake of completeness
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] RedMickey commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
RedMickey commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-731141598
> The problem I see with this PR: Until End of March 2021 the old browser is still supported by MS - is there a clever way to support both browsers? Otherwise this PR marks a breaking change.
Hi @timbru31, I rolled back some changes, so now the package supports both Edge browsers (old and chromium based).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] seamlink-aalves commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
seamlink-aalves commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-978101034
Any chance of having this merged and a new version created?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] RedMickey commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
RedMickey commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-830096056
Hi @erisu, @timbru31. Could you please take a look at this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-serve] timbru31 commented on pull request #40: Add MS Edge Chromium support
Posted by GitBox <gi...@apache.org>.
timbru31 commented on pull request #40:
URL: https://github.com/apache/cordova-serve/pull/40#issuecomment-730228759
The problem I see with this PR: Until End of March 2021 the old browser is still supported by MS - is there a clever way to support both browsers? Otherwise this PR marks a breaking change.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org