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

Issue 29536764: Issue 5587, 5748 - Use mobile options page on Firefox for Android (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 2 months ago by Manish Jethani
Modified:
2 years, 1 month ago
CC:
kzar
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Patch Set 1 #

Total comments: 30

Patch Set 2 : Move browserAction handler to lib/browserAction.js #

Total comments: 6

Patch Set 3 : Add options page loader #

Total comments: 5

Patch Set 4 : Use iframe instead of redirection #

Patch Set 5 : Move file names to HTML #

Patch Set 6 : Pass boolean instead of object #

Total comments: 2

Patch Set 7 : Use runtime.openOptionsPage on Firefox 57+ #

Patch Set 8 : Forward query string and fragment identifier to iframe #

Total comments: 9

Patch Set 9 : Query tabs on Firefox as well #

Total comments: 2

Patch Set 10 : Queue up messages from background page and forward them later #

Total comments: 19

Patch Set 11 : Update title, add defer attribute, coding style #

Total comments: 3

Patch Set 12 : Use ping messages to sync up #

Total comments: 13

Patch Set 13 : Rebase on next #

Total comments: 10

Patch Set 14 : Wait for app.listen #

Total comments: 3

Patch Set 15 : Update comment about relative URL #

Total comments: 9

Patch Set 16 : Merge lib/browserAction.js into lib/options.js #

Total comments: 11

Patch Set 17 : Remove ping and handle error #

Total comments: 7

Patch Set 18 : Do not callback if tab not found #

Patch Set 19 : Set content size #

Total comments: 3

Patch Set 20 : Remove redundant top and left properties #

Patch Set 21 : Rebase #

Total comments: 3

Patch Set 22 : Hide content frame until loaded #

Total comments: 8

Patch Set 23 : Remove workaround for FOUC issue and update adblockplusui dependency #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+205 lines, -1101 lines) Patch
M dependencies View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +1 line, -1 line 0 comments Download
M desktop-options.html View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M desktop-options.js View 1 2 3 12 13 14 15 16 0 chunks +-1 lines, --1 lines 0 comments Download
M ext/background.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +0 lines, -88 lines 0 comments Download
M lib/notificationHelper.js View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +2 lines, -1 line 0 comments Download
A lib/options.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +152 lines, -0 lines 0 comments Download
M metadata.chrome View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +2 lines, -1 line 0 comments Download
M metadata.gecko-webext View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +12 lines, -0 lines 0 comments Download
M options.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 22 1 chunk +22 lines, -286 lines 0 comments Download
M options.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 22 1 chunk +12 lines, -723 lines 3 comments Download
M popup.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 79
Manish Jethani
2 years, 2 months ago (2017-09-06 01:05:30 UTC) #1
Manish Jethani
Patch Set 1 See inline comments. https://codereview.adblockplus.org/29536764/diff/29536765/dependencies File dependencies (right): https://codereview.adblockplus.org/29536764/diff/29536765/dependencies#newcode6 dependencies:6: adblockplusui = adblockplusui ...
2 years, 2 months ago (2017-09-06 01:12:13 UTC) #2
Manish Jethani
Added Ollie. Ollie, does this look OK: https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#oldcode738
2 years, 2 months ago (2017-09-06 01:19:48 UTC) #3
Oleksandr
LGTM https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#newcode790 ext/background.js:790: ext.pages.open(optionsUrl, callback); Maybe add a comment above this ...
2 years, 2 months ago (2017-09-06 02:51:58 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#oldcode322 ext/background.js:322: if (!("getPopup" in chrome.browserAction)) Is this check still correct? ...
2 years, 2 months ago (2017-09-06 03:17:13 UTC) #5
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#oldcode322 ext/background.js:322: if (!("getPopup" in chrome.browserAction)) On 2017/09/06 03:17:12, Sebastian Noack ...
2 years, 2 months ago (2017-09-06 10:32:23 UTC) #6
Thomas Greiner
https://codereview.adblockplus.org/29536764/diff/29536765/dependencies File dependencies (right): https://codereview.adblockplus.org/29536764/diff/29536765/dependencies#newcode6 dependencies:6: adblockplusui = adblockplusui hg:8ad6a38e6870 git:9460adb On 2017/09/06 01:12:12, Manish ...
2 years, 2 months ago (2017-09-06 12:40:03 UTC) #7
Thomas Greiner
https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#newcode345 ext/background.js:345: .checkWhitelisted(currentPage) FYI: I've created a spec ticket to get ...
2 years, 2 months ago (2017-09-06 13:06:35 UTC) #8
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#oldcode705 ext/background.js:705: // Some versions of Firefox for Android before version ...
2 years, 2 months ago (2017-09-07 02:53:39 UTC) #9
Manish Jethani
Patch Set 2: Move browserAction handler to lib/browserAction.js I haven't addressed the issue of options.html ...
2 years, 2 months ago (2017-09-07 11:04:17 UTC) #10
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#newcode345 ext/background.js:345: .checkWhitelisted(currentPage) On 2017/09/06 13:06:34, Thomas Greiner wrote: > FYI: ...
2 years, 2 months ago (2017-09-07 11:12:55 UTC) #11
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js File ext/background.js (left): https://codereview.adblockplus.org/29536764/diff/29536765/ext/background.js#oldcode705 ext/background.js:705: // Some versions of Firefox for Android before version ...
2 years, 2 months ago (2017-09-07 16:36:46 UTC) #12
Manish Jethani
Patch Set 3: Add options page loader https://codereview.adblockplus.org/29536764/diff/29538606/lib/browserAction.js File lib/browserAction.js (right): https://codereview.adblockplus.org/29536764/diff/29538606/lib/browserAction.js#newcode35 lib/browserAction.js:35: if (!["http:", ...
2 years, 2 months ago (2017-09-08 10:02:25 UTC) #13
Manish Jethani
Patch Set 4: Use iframe instead of redirection I like this approach because there's no ...
2 years, 2 months ago (2017-09-08 10:46:46 UTC) #14
Manish Jethani
Patch Set 5: Move file names to HTML Slightly better as the file names are ...
2 years, 2 months ago (2017-09-08 12:45:48 UTC) #15
Manish Jethani
Patch Set 6: Pass boolean instead of object Fixed a bug here, it as passing ...
2 years, 2 months ago (2017-09-08 12:50:53 UTC) #16
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29538606/lib/browserAction.js File lib/browserAction.js (right): https://codereview.adblockplus.org/29536764/diff/29538606/lib/browserAction.js#newcode35 lib/browserAction.js:35: if (!["http:", "https:"].includes(currentPage.url.protocol)) On 2017/09/08 10:02:24, Manish Jethani wrote: ...
2 years, 2 months ago (2017-09-11 22:40:02 UTC) #17
Manish Jethani
Patch Set 7: Use runtime.openOptionsPage on Firefox 57+ https://codereview.adblockplus.org/29536764/diff/29539735/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29536764/diff/29539735/ext/background.js#newcode697 ext/background.js:697: info.application ...
2 years, 2 months ago (2017-09-12 12:39:18 UTC) #18
Sebastian Noack
LGTM (for "next" bookmark)
2 years, 2 months ago (2017-09-12 21:04:01 UTC) #19
Manish Jethani
Patch Set 8: Forward query string and fragment identifier to iframe According to adblockplusui/README.md the ...
2 years, 2 months ago (2017-09-13 00:28:30 UTC) #20
Sebastian Noack
On 2017/09/13 00:28:30, Manish Jethani wrote: > Patch Set 8: Forward query string and fragment ...
2 years, 2 months ago (2017-09-13 00:45:51 UTC) #21
Manish Jethani
On 2017/09/13 00:45:51, Sebastian Noack wrote: > On 2017/09/13 00:28:30, Manish Jethani wrote: > > ...
2 years, 2 months ago (2017-09-13 00:57:00 UTC) #22
Thomas Greiner
On 2017/09/13 00:57:00, Manish Jethani wrote: > Thomas, would you like to take a look ...
2 years, 2 months ago (2017-09-13 13:44:44 UTC) #23
Thomas Greiner
I like the approach you use of having interstitial page for doing the redirect. On ...
2 years, 2 months ago (2017-09-13 14:56:32 UTC) #24
Manish Jethani
Patch Set 9: Query tabs on Firefox as well Comments inline. https://codereview.adblockplus.org/29536764/diff/29542849/ext/background.js File ext/background.js (right): ...
2 years, 2 months ago (2017-09-13 16:07:45 UTC) #25
Sebastian Noack
On 2017/09/13 14:56:32, Thomas Greiner wrote: > I agree that it's probably a good idea ...
2 years, 2 months ago (2017-09-13 16:43:24 UTC) #26
Thomas Greiner
https://codereview.adblockplus.org/29536764/diff/29542849/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29542849/options.js#newcode29 options.js:29: iframe.src = src; On 2017/09/13 16:07:43, Manish Jethani wrote: ...
2 years, 2 months ago (2017-09-13 16:46:44 UTC) #27
Thomas Greiner
@Sebastian Sorry, I didn't see your recent comment when I replied but yeah, while it'd ...
2 years, 2 months ago (2017-09-13 16:57:11 UTC) #28
Sebastian Noack
On 2017/09/13 16:57:11, Thomas Greiner wrote: > Regarding the second part of your comment, please ...
2 years, 2 months ago (2017-09-13 17:57:52 UTC) #29
Manish Jethani
On 2017/09/13 17:57:52, Sebastian Noack wrote: > We could, however, limit that effect to mobile ...
2 years, 2 months ago (2017-09-14 03:30:36 UTC) #30
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29542849/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29542849/options.js#newcode29 options.js:29: iframe.src = src; On 2017/09/13 16:46:44, Thomas Greiner wrote: ...
2 years, 2 months ago (2017-09-14 04:13:51 UTC) #31
Manish Jethani
On 2017/09/13 16:57:11, Thomas Greiner wrote: > Regarding the second part of your comment, please ...
2 years, 2 months ago (2017-09-14 04:16:19 UTC) #32
Thomas Greiner
On 2017/09/14 03:30:36, Manish Jethani wrote: > We would use > location.replace [1] here. Unfortunately ...
2 years, 2 months ago (2017-09-14 10:13:41 UTC) #33
Manish Jethani
On 2017/09/14 10:13:41, Thomas Greiner wrote: > On 2017/09/14 03:30:36, Manish Jethani wrote: > > ...
2 years, 2 months ago (2017-09-15 09:49:24 UTC) #34
Manish Jethani
On 2017/09/15 09:49:24, Manish Jethani wrote: > The third one is even more complicated, it ...
2 years, 2 months ago (2017-09-15 09:54:04 UTC) #35
Thomas Greiner
On 2017/09/15 09:49:24, Manish Jethani wrote: > Anyway, this is getting way too complicated now ...
2 years, 2 months ago (2017-09-15 12:13:03 UTC) #36
Manish Jethani
Patch Set 10: Queue up messages from background page and forward them later This addresses ...
2 years, 2 months ago (2017-09-15 17:57:29 UTC) #37
Thomas Greiner
Looks good overall. Just a few small remarks since we've now concluded that the iFrame ...
2 years, 1 month ago (2017-09-19 17:51:21 UTC) #38
Manish Jethani
Patch Set 11: Update title, add defer attribute, coding style https://codereview.adblockplus.org/29536764/diff/29545684/options.html File options.html (left): https://codereview.adblockplus.org/29536764/diff/29545684/options.html#oldcode30 ...
2 years, 1 month ago (2017-09-19 19:42:07 UTC) #39
Thomas Greiner
LGTM https://codereview.adblockplus.org/29536764/diff/29545684/options.html File options.html (left): https://codereview.adblockplus.org/29536764/diff/29545684/options.html#oldcode30 options.html:30: <title>Adblock Plus Options</title> On 2017/09/19 19:42:04, Manish Jethani ...
2 years, 1 month ago (2017-09-20 12:30:00 UTC) #40
Manish Jethani
On 2017/09/20 12:30:00, Thomas Greiner wrote: > LGTM Thanks. Sebastian brought up on IRC that ...
2 years, 1 month ago (2017-09-20 12:50:15 UTC) #41
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29545684/options.html File options.html (left): https://codereview.adblockplus.org/29536764/diff/29545684/options.html#oldcode30 options.html:30: <title>Adblock Plus Options</title> On 2017/09/20 12:29:59, Thomas Greiner wrote: ...
2 years, 1 month ago (2017-09-20 12:50:28 UTC) #42
Manish Jethani
Patch Set 12: Use ping messages to sync up https://codereview.adblockplus.org/29536764/diff/29549910/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29536764/diff/29549910/ext/background.js#newcode690 ext/background.js:690: ...
2 years, 1 month ago (2017-09-26 20:37:20 UTC) #43
Manish Jethani
Some more comments ... https://codereview.adblockplus.org/29536764/diff/29556686/lib/options.js File lib/options.js (right): https://codereview.adblockplus.org/29536764/diff/29556686/lib/options.js#newcode25 lib/options.js:25: function whenOnline(page) There should probably ...
2 years, 1 month ago (2017-09-26 20:53:05 UTC) #44
Manish Jethani
Patch Set 13: Rebase on next
2 years, 1 month ago (2017-09-26 22:05:21 UTC) #45
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29556686/desktop-options.js File desktop-options.js (right): https://codereview.adblockplus.org/29536764/diff/29556686/desktop-options.js#newcode749 desktop-options.js:749: $(() => On 2017/09/26 20:37:16, Manish Jethani wrote: > ...
2 years, 1 month ago (2017-09-27 01:38:36 UTC) #46
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29556686/desktop-options.js File desktop-options.js (right): https://codereview.adblockplus.org/29536764/diff/29556686/desktop-options.js#newcode749 desktop-options.js:749: $(() => On 2017/09/27 01:38:34, Sebastian Noack wrote: > ...
2 years, 1 month ago (2017-09-27 11:20:56 UTC) #47
Manish Jethani
Patch Set 14: Wait for app.listen https://codereview.adblockplus.org/29536764/diff/29557653/lib/options.js File lib/options.js (right): https://codereview.adblockplus.org/29536764/diff/29557653/lib/options.js#newcode69 lib/options.js:69: let page = ...
2 years, 1 month ago (2017-09-27 12:50:15 UTC) #48
Manish Jethani
Patch Set 15: Update comment about relative URL
2 years, 1 month ago (2017-09-27 12:57:09 UTC) #49
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29556686/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29556686/options.js#newcode20 options.js:20: const {require} = chrome.extension.getBackgroundPage(); On 2017/09/27 11:20:54, Manish Jethani ...
2 years, 1 month ago (2017-09-30 02:38:46 UTC) #50
Manish Jethani
Patch Set 16: Merge lib/browserAction.js into lib/options.js https://codereview.adblockplus.org/29536764/diff/29556686/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29556686/options.js#newcode20 options.js:20: const {require} ...
2 years, 1 month ago (2017-09-30 13:49:55 UTC) #51
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29560558/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29560558/options.js#newcode27 options.js:27: chrome.runtime.sendMessage({ I should note that there's a layout issue ...
2 years, 1 month ago (2017-09-30 14:23:34 UTC) #52
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29556757/lib/notificationHelper.js File lib/notificationHelper.js (right): https://codereview.adblockplus.org/29536764/diff/29556757/lib/notificationHelper.js#newcode127 lib/notificationHelper.js:127: showOptions(page => On 2017/09/30 13:49:52, Manish Jethani wrote: > ...
2 years, 1 month ago (2017-10-02 01:47:08 UTC) #53
Manish Jethani
Patch Set 17: Remove ping and handle error https://codereview.adblockplus.org/29536764/diff/29556757/lib/notificationHelper.js File lib/notificationHelper.js (right): https://codereview.adblockplus.org/29536764/diff/29556757/lib/notificationHelper.js#newcode127 lib/notificationHelper.js:127: showOptions(page ...
2 years, 1 month ago (2017-10-02 08:03:15 UTC) #54
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29560558/options.html File options.html (right): https://codereview.adblockplus.org/29536764/diff/29560558/options.html#newcode39 options.html:39: width: 100%; On 2017/10/02 08:03:14, Manish Jethani wrote: > ...
2 years, 1 month ago (2017-10-04 01:22:34 UTC) #55
Manish Jethani
Patch Set 18: Do not callback if tab not found https://codereview.adblockplus.org/29536764/diff/29560558/options.html File options.html (right): https://codereview.adblockplus.org/29536764/diff/29560558/options.html#newcode39 ...
2 years, 1 month ago (2017-10-04 03:46:50 UTC) #56
Manish Jethani
Patch Set 19: Set content size https://codereview.adblockplus.org/29536764/diff/29560558/options.html File options.html (right): https://codereview.adblockplus.org/29536764/diff/29560558/options.html#newcode39 options.html:39: width: 100%; On ...
2 years, 1 month ago (2017-10-04 03:57:16 UTC) #57
Sebastian Noack
Looks good to me except for one small nit. Thanks! https://codereview.adblockplus.org/29536764/diff/29560558/options.html File options.html (right): https://codereview.adblockplus.org/29536764/diff/29560558/options.html#newcode39 ...
2 years, 1 month ago (2017-10-04 05:33:17 UTC) #58
Manish Jethani
Patch Set 20: Remove redundant top and left properties https://codereview.adblockplus.org/29536764/diff/29562555/lib/options.js File lib/options.js (right): https://codereview.adblockplus.org/29536764/diff/29562555/lib/options.js#newcode65 lib/options.js:65: ...
2 years, 1 month ago (2017-10-04 13:02:48 UTC) #59
Manish Jethani
Ollie, would you like to take a look, esp. in lib/options.js where it affects Edge ...
2 years, 1 month ago (2017-10-04 13:07:22 UTC) #60
Thomas Greiner
LGTM
2 years, 1 month ago (2017-10-04 15:50:53 UTC) #61
Sebastian Noack
LGTM On 2017/10/04 13:07:22, Manish Jethani wrote: > Ollie, would you like to take a ...
2 years, 1 month ago (2017-10-04 19:00:22 UTC) #62
Manish Jethani
Patch Set 21: Rebase One change. https://codereview.adblockplus.org/29536764/diff/29564724/popup.js File popup.js (right): https://codereview.adblockplus.org/29536764/diff/29564724/popup.js#newcode125 popup.js:125: chrome.runtime.sendMessage({type: "app.open", what: ...
2 years, 1 month ago (2017-10-04 20:21:48 UTC) #63
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29564724/popup.js File popup.js (right): https://codereview.adblockplus.org/29536764/diff/29564724/popup.js#newcode126 popup.js:126: window.close(); Did you test whether the message is still ...
2 years, 1 month ago (2017-10-04 21:38:50 UTC) #64
Manish Jethani
Patch Set 22: Hide content frame until loaded https://codereview.adblockplus.org/29536764/diff/29564724/popup.js File popup.js (right): https://codereview.adblockplus.org/29536764/diff/29564724/popup.js#newcode126 popup.js:126: window.close(); ...
2 years, 1 month ago (2017-10-04 22:03:53 UTC) #65
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29564756/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29564756/options.js#newcode28 options.js:28: chrome.runtime.sendMessage({ On 2017/10/04 22:03:51, Manish Jethani wrote: > After ...
2 years, 1 month ago (2017-10-04 22:05:59 UTC) #66
Sebastian Noack
https://codereview.adblockplus.org/29536764/diff/29564756/options.html File options.html (right): https://codereview.adblockplus.org/29536764/diff/29564756/options.html#newcode42 options.html:42: <iframe id="content" style="visibility: hidden" data-src="desktop-options.html" data-src-fennec="mobile-options.html"></iframe> Using style attributes ...
2 years, 1 month ago (2017-10-04 22:36:20 UTC) #67
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29564756/options.html File options.html (right): https://codereview.adblockplus.org/29536764/diff/29564756/options.html#newcode42 options.html:42: <iframe id="content" style="visibility: hidden" data-src="desktop-options.html" data-src-fennec="mobile-options.html"></iframe> On 2017/10/04 22:36:19, ...
2 years, 1 month ago (2017-10-04 23:07:53 UTC) #68
Sebastian Noack
LGTM
2 years, 1 month ago (2017-10-04 23:30:25 UTC) #69
Thomas Greiner
https://codereview.adblockplus.org/29536764/diff/29564756/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29564756/options.js#newcode28 options.js:28: chrome.runtime.sendMessage({ On 2017/10/04 22:03:51, Manish Jethani wrote: > After ...
2 years, 1 month ago (2017-10-05 11:51:44 UTC) #70
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29564756/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29564756/options.js#newcode28 options.js:28: chrome.runtime.sendMessage({ On 2017/10/05 11:51:43, Thomas Greiner wrote: > On ...
2 years, 1 month ago (2017-10-05 12:14:27 UTC) #71
Thomas Greiner
https://codereview.adblockplus.org/29536764/diff/29564756/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29564756/options.js#newcode28 options.js:28: chrome.runtime.sendMessage({ On 2017/10/05 12:14:25, Manish Jethani wrote: > On ...
2 years, 1 month ago (2017-10-05 12:34:37 UTC) #72
Manish Jethani
Patch Set 23: Remove workaround for FOUC issue and update adblockplusui dependency https://codereview.adblockplus.org/29536764/diff/29564756/options.js File options.js ...
2 years, 1 month ago (2017-10-05 13:26:31 UTC) #73
Thomas Greiner
LGTM
2 years, 1 month ago (2017-10-05 13:33:39 UTC) #74
Sebastian Noack
LGTM
2 years, 1 month ago (2017-10-05 16:31:30 UTC) #75
Manish Jethani
Thanks, folks!
2 years, 1 month ago (2017-10-05 16:32:57 UTC) #76
Oleksandr
https://codereview.adblockplus.org/29536764/diff/29565705/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29565705/options.js#newcode27 options.js:27: chrome.runtime.sendMessage({ Edge does not support the 'chrome' namespace for ...
2 years, 1 month ago (2017-10-09 11:50:00 UTC) #77
Manish Jethani
https://codereview.adblockplus.org/29536764/diff/29565705/options.js File options.js (right): https://codereview.adblockplus.org/29536764/diff/29565705/options.js#newcode27 options.js:27: chrome.runtime.sendMessage({ On 2017/10/09 11:49:59, Oleksandr wrote: > Edge does ...
2 years, 1 month ago (2017-10-09 12:00:56 UTC) #78
Sebastian Noack
2 years, 1 month ago (2017-10-09 15:09:58 UTC) #79
Message was sent while issue was closed.
https://codereview.adblockplus.org/29536764/diff/29565705/options.js
File options.js (right):

https://codereview.adblockplus.org/29536764/diff/29565705/options.js#newcode27
options.js:27: chrome.runtime.sendMessage({
On 2017/10/09 12:00:55, Manish Jethani wrote:
> On 2017/10/09 11:49:59, Oleksandr wrote:
> > Edge does not support the 'chrome' namespace for extensions. We should map
to
> > 'browser' here instead.
> 
> Thanks.

Note that we already do so in ext/common.js:
https://hg.adblockplus.org/adblockpluschrome/file/7136ad8e321d/ext/common.js#l22
Sign in to reply to this message.

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