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

Issue 29570614: Issue 5028 - Use browser namespace (Closed)

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

Description

Related: https://codereview.adblockplus.org/29573083/ Related: https://codereview.adblockplus.org/29573134/

Patch Set 1 #

Total comments: 7

Patch Set 2 : Fix indentation #

Patch Set 3 : Use callback-based API for storage #

Patch Set 4 : Rebase #

Patch Set 5 : Use polyfill.js #

Patch Set 6 : Refactor polyfill.js #

Total comments: 11

Patch Set 7 : Promisify #

Total comments: 1

Patch Set 8 : Use browser object directly when available #

Patch Set 9 : Rebase #

Patch Set 10 : Rebase #

Total comments: 2

Patch Set 11 : Update buildtools dependency #

Patch Set 12 : Update ensure_dependencies.py #

Patch Set 13 : Update adblockplusui dependency #

Unified diffs Side-by-side diffs Delta from patch set Stats (+269 lines, -227 lines) Patch
M composer.html View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M composer.js View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -2 lines 0 comments Download
M composer.postload.js View 1 2 3 4 5 6 7 8 9 5 chunks +8 lines, -8 lines 0 comments Download
M dependencies View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +2 lines, -2 lines 0 comments Download
M desktop-options.html View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M desktop-options.js View 1 2 3 4 5 6 7 8 9 7 chunks +12 lines, -12 lines 0 comments Download
M devtools.html View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M devtools.js View 1 chunk +3 lines, -3 lines 0 comments Download
M ensure_dependencies.py View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +14 lines, -2 lines 0 comments Download
M ext/background.js View 10 chunks +53 lines, -53 lines 0 comments Download
M ext/common.js View 1 2 3 4 5 6 7 8 9 2 chunks +0 lines, -18 lines 0 comments Download
M ext/content.js View 1 chunk +5 lines, -5 lines 0 comments Download
M ext/devtools.js View 1 chunk +3 lines, -3 lines 0 comments Download
M include.preload.js View 1 2 3 4 5 6 7 8 9 4 chunks +7 lines, -7 lines 0 comments Download
M inject.preload.js View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M lib/csp.js View 1 chunk +3 lines, -3 lines 0 comments Download
M lib/cssInjection.js View 1 chunk +3 lines, -3 lines 0 comments Download
M lib/devtools.js View 3 chunks +4 lines, -4 lines 0 comments Download
M lib/filterComposer.js View 1 2 3 4 5 6 7 8 3 chunks +3 lines, -3 lines 0 comments Download
M lib/filterValidation.js View 1 chunk +2 lines, -2 lines 0 comments Download
M lib/firefoxDataCleanup.js View 1 2 1 chunk +1 line, -1 line 0 comments Download
M lib/icon.js View 2 chunks +2 lines, -2 lines 0 comments Download
M lib/io.js View 2 chunks +4 lines, -4 lines 0 comments Download
M lib/notificationHelper.js View 1 2 3 4 5 6 7 8 9 10 5 chunks +24 lines, -16 lines 0 comments Download
M lib/options.js View 1 2 3 4 5 6 7 8 2 chunks +10 lines, -10 lines 0 comments Download
M lib/popupBlocker.js View 3 chunks +11 lines, -11 lines 0 comments Download
M lib/prefs.js View 1 2 3 1 chunk +5 lines, -5 lines 0 comments Download
M lib/requestBlocker.js View 3 chunks +7 lines, -7 lines 0 comments Download
M lib/stats.js View 1 chunk +1 line, -1 line 0 comments Download
M lib/subscriptionInit.js View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
M lib/uninstall.js View 1 1 chunk +3 lines, -3 lines 0 comments Download
M lib/utils.js View 2 chunks +3 lines, -3 lines 0 comments Download
M lib/whitelisting.js View 2 chunks +3 lines, -3 lines 0 comments Download
M metadata.chrome View 1 2 3 4 2 chunks +5 lines, -3 lines 0 comments Download
M options.html View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M options.js View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
A polyfill.js View 1 2 3 4 5 6 7 1 chunk +33 lines, -0 lines 0 comments Download
M popup.html View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M popup.js View 1 2 3 10 chunks +22 lines, -22 lines 0 comments Download
M qunit/tests/prefs.js View 1 chunk +2 lines, -2 lines 0 comments Download
M subscriptionLink.postload.js View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 36
Manish Jethani
2 years, 1 month ago (2017-10-09 15:39:54 UTC) #1
Manish Jethani
Patch Set 1
2 years, 1 month ago (2017-10-09 15:42:06 UTC) #2
Manish Jethani
https://codereview.adblockplus.org/29570614/diff/29570615/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29570614/diff/29570615/ext/background.js#newcode167 ext/background.js:167: // tabs. However, we have to keep relying on ...
2 years, 1 month ago (2017-10-09 15:49:41 UTC) #3
Manish Jethani
Patch Set 2: Fix indentation
2 years, 1 month ago (2017-10-09 15:53:45 UTC) #4
Manish Jethani
Patch Set 3: Use callback-based API for storage
2 years, 1 month ago (2017-10-09 16:03:53 UTC) #5
Sebastian Noack
BTW, this file in buildtools should be updated too: https://hg.adblockplus.org/buildtools/file/d4ca9bfb82b5/chromeDevenvPoller__.js https://codereview.adblockplus.org/29570614/diff/29570615/options.js File options.js (right): https://codereview.adblockplus.org/29570614/diff/29570615/options.js#newcode20 ...
2 years, 1 month ago (2017-10-09 16:05:38 UTC) #6
Manish Jethani
On 2017/10/09 16:05:38, Sebastian Noack wrote: > BTW, this file in buildtools should be updated ...
2 years, 1 month ago (2017-10-09 16:08:31 UTC) #7
Manish Jethani
https://codereview.adblockplus.org/29570614/diff/29570615/options.js File options.js (right): https://codereview.adblockplus.org/29570614/diff/29570615/options.js#newcode20 options.js:20: // Both Edge and Mozilla Web Extensions use the ...
2 years, 1 month ago (2017-10-09 16:08:39 UTC) #8
Sebastian Noack
https://codereview.adblockplus.org/29570614/diff/29570615/options.js File options.js (right): https://codereview.adblockplus.org/29570614/diff/29570615/options.js#newcode20 options.js:20: // Both Edge and Mozilla Web Extensions use the ...
2 years, 1 month ago (2017-10-09 16:14:19 UTC) #9
Manish Jethani
Patch Set 5: Use polyfill.js There are corresponding changes in adblockplusui
2 years, 1 month ago (2017-10-10 22:17:19 UTC) #10
Manish Jethani
On 2017/10/10 22:17:19, Manish Jethani wrote: > Patch Set 5: Use polyfill.js > > There ...
2 years, 1 month ago (2017-10-10 22:26:39 UTC) #11
Manish Jethani
Patch Set 6: Refactor polyfill.js
2 years, 1 month ago (2017-10-10 22:40:41 UTC) #12
Manish Jethani
On 2017/10/09 16:05:38, Sebastian Noack wrote: > BTW, this file in buildtools should be updated ...
2 years, 1 month ago (2017-10-10 22:58:45 UTC) #13
Sebastian Noack
https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension APIs ...
2 years, 1 month ago (2017-10-10 23:07:44 UTC) #14
Sebastian Noack
https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension APIs ...
2 years, 1 month ago (2017-10-10 23:15:25 UTC) #15
Manish Jethani
https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension APIs ...
2 years, 1 month ago (2017-10-11 01:09:20 UTC) #16
Sebastian Noack
https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension APIs ...
2 years, 1 month ago (2017-10-11 01:40:06 UTC) #17
Manish Jethani
Patch Set 7: Promisify First draft of the promisified implementation. This brings the browser namespace ...
2 years, 1 month ago (2017-10-11 03:26:44 UTC) #18
Sebastian Noack
On 2017/10/11 03:26:44, Manish Jethani wrote: > Patch Set 7: Promisify > > First draft ...
2 years, 1 month ago (2017-10-11 03:45:33 UTC) #19
Manish Jethani
On 2017/10/11 03:45:33, Sebastian Noack wrote: > > While I'm all for enabling a progressive ...
2 years, 1 month ago (2017-10-11 10:46:41 UTC) #20
Manish Jethani
Patch Set 8: Use browser object directly when available https://codereview.adblockplus.org/29570614/diff/29570615/options.js File options.js (right): https://codereview.adblockplus.org/29570614/diff/29570615/options.js#newcode20 options.js:20: ...
2 years, 1 month ago (2017-10-11 10:59:20 UTC) #21
Manish Jethani
https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension APIs ...
2 years, 1 month ago (2017-10-11 18:36:12 UTC) #22
Sebastian Noack
LGTM https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension ...
2 years, 1 month ago (2017-10-13 02:59:49 UTC) #23
Manish Jethani
On 2017/10/13 02:59:49, Sebastian Noack wrote: > LGTM Thanks!
2 years, 1 month ago (2017-10-13 04:27:21 UTC) #24
Manish Jethani
https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js File polyfill.js (right): https://codereview.adblockplus.org/29570614/diff/29573098/polyfill.js#newcode28 polyfill.js:28: // for a "chrome" namespace with the extension APIs ...
2 years, 1 month ago (2017-10-13 04:27:31 UTC) #25
Manish Jethani
Patch Set 9: Rebase
2 years, 1 month ago (2017-10-13 04:55:38 UTC) #26
Sebastian Noack
LGTM
2 years, 1 month ago (2017-10-13 05:21:55 UTC) #27
Manish Jethani
Patch Set 10: Rebase
2 years, 1 month ago (2017-10-13 20:01:02 UTC) #28
Sebastian Noack
Still LGTM
2 years, 1 month ago (2017-10-13 23:17:38 UTC) #29
kzar
As mentioned in IRC I recommend you update the buildtools dependency to ba74948f8f01 as part ...
2 years, 1 month ago (2017-10-15 12:45:34 UTC) #30
kzar
I've tested out these changes along with my webpack ones. They mostly seem to work ...
2 years, 1 month ago (2017-10-16 10:35:52 UTC) #31
Manish Jethani
Patch Set 11: Update buildtools dependency https://codereview.adblockplus.org/29570614/diff/29575696/lib/notificationHelper.js File lib/notificationHelper.js (right): https://codereview.adblockplus.org/29570614/diff/29575696/lib/notificationHelper.js#newcode215 lib/notificationHelper.js:215: "notification_without_buttons"); On 2017/10/16 ...
2 years, 1 month ago (2017-10-16 12:17:36 UTC) #32
Manish Jethani
Patch Set 12: Update ensure_dependencies.py
2 years, 1 month ago (2017-10-16 12:18:10 UTC) #33
Manish Jethani
On 2017/10/16 10:35:52, kzar wrote: > I've tested out these changes along with my webpack ...
2 years, 1 month ago (2017-10-16 12:19:33 UTC) #34
kzar
LGTM once adblockplusui dependency is updated.
2 years, 1 month ago (2017-10-16 12:24:22 UTC) #35
Manish Jethani
2 years, 1 month ago (2017-10-17 13:11:51 UTC) #36
On 2017/10/16 12:24:22, kzar wrote:
> LGTM once adblockplusui dependency is updated.

Thanks!

https://hg.adblockplus.org/adblockpluschrome/rev/f9dfddb26058
Sign in to reply to this message.

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