Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(51)

Issue 29371945: Issue 4802 - Add back compatibility for Microsoft Edge after #4722 removed it (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 1 month ago by Oleksandr
Modified:
3 years, 1 month ago
Reviewers:
kzar, Sebastian Noack
Visibility:
Public.

Description

This adds back the fetch polyfill and makes sure we don't call sendMessage with undefined callback. Also updates the buildtools dependency to the latest version.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Don't touch the buildtools dependency #

Total comments: 3

Patch Set 3 : Add a link to an Edge issue #

Patch Set 4 : Specify the version of Edge where the issue is observed #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -2 lines) Patch
M metadata.common View 1 chunk +2 lines, -1 line 0 comments Download
M options.html View 1 chunk +1 line, -0 lines 0 comments Download
M options.js View 1 2 3 1 chunk +7 lines, -1 line 2 comments Download

Messages

Total messages: 14
Oleksandr
3 years, 1 month ago (2017-01-16 06:40:08 UTC) #1
kzar
Is this intended for the master branch or the edge branch? I don't mind most ...
3 years, 1 month ago (2017-01-16 07:27:30 UTC) #2
Oleksandr
On 2017/01/16 07:27:30, kzar wrote: > Is this intended for the master branch or the ...
3 years, 1 month ago (2017-01-16 08:21:13 UTC) #3
Oleksandr
https://codereview.adblockplus.org/29371945/diff/29371946/dependencies File dependencies (left): https://codereview.adblockplus.org/29371945/diff/29371946/dependencies#oldcode3 dependencies:3: buildtools = buildtools hg:4cea55c4bdc0 git:33dfff3 On 2017/01/16 07:27:30, kzar ...
3 years, 1 month ago (2017-01-16 08:21:37 UTC) #4
kzar
On 2017/01/16 08:21:13, Oleksandr wrote: > This is intended for the edge bookmark. Not following ...
3 years, 1 month ago (2017-01-16 08:26:12 UTC) #5
Oleksandr
Remove the buildtools dependcy update in patchset 2
3 years, 1 month ago (2017-01-17 05:47:11 UTC) #6
Oleksandr
On 2017/01/16 08:26:12, kzar wrote: > Well you should re-open the issue for the last ...
3 years, 1 month ago (2017-01-17 06:00:38 UTC) #7
kzar
Otherwise LGTM https://codereview.adblockplus.org/29371945/diff/29372118/options.js File options.js (right): https://codereview.adblockplus.org/29371945/diff/29372118/options.js#newcode59 options.js:59: // Edge silently fails when sendMessage is ...
3 years, 1 month ago (2017-01-17 06:54:31 UTC) #8
Oleksandr
https://codereview.adblockplus.org/29371945/diff/29372118/options.js File options.js (right): https://codereview.adblockplus.org/29371945/diff/29372118/options.js#newcode59 options.js:59: // Edge silently fails when sendMessage is called with ...
3 years, 1 month ago (2017-01-18 03:08:44 UTC) #9
kzar
https://codereview.adblockplus.org/29371945/diff/29372118/options.js File options.js (right): https://codereview.adblockplus.org/29371945/diff/29372118/options.js#newcode59 options.js:59: // Edge silently fails when sendMessage is called with ...
3 years, 1 month ago (2017-01-18 03:48:41 UTC) #10
Oleksandr
Done. Pushing this.
3 years, 1 month ago (2017-01-18 03:58:44 UTC) #11
kzar
https://codereview.adblockplus.org/29371945/diff/29372257/options.js File options.js (right): https://codereview.adblockplus.org/29371945/diff/29372257/options.js#newcode60 options.js:60: // parameter of null, so we work around that ...
3 years, 1 month ago (2017-01-18 04:49:51 UTC) #12
Oleksandr
https://codereview.adblockplus.org/29371945/diff/29372257/options.js File options.js (right): https://codereview.adblockplus.org/29371945/diff/29372257/options.js#newcode60 options.js:60: // parameter of null, so we work around that ...
3 years, 1 month ago (2017-01-19 20:51:01 UTC) #13
kzar
3 years, 1 month ago (2017-01-20 09:58:30 UTC) #14
Message was sent while issue was closed.
On 2017/01/19 20:51:01, Oleksandr wrote:
> Fixed that in a followup commit.

Cool, thanks.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5