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

Issue 29460576: Issue 5079 - Turn elemHideEmulation into a CommonJS module (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 8 months ago by hub
Modified:
2 years, 6 months ago
Base URL:
https://hg.adblockplus.org/adblockpluscore/
Visibility:
Public.

Description

Issue 5079 - Turn elemHideEmulation into a CommonJS module Applies on top of https://codereview.adblockplus.org/29448560/

Patch Set 1 #

Total comments: 12

Patch Set 2 : Added the eslintrc #

Total comments: 2

Patch Set 3 : Rebased. No longer need browser env #

Patch Set 4 : Updated patch. Fails for now #

Patch Set 5 : Rebased on master #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -10 lines) Patch
R chrome/content/.eslintrc.json View 1 2 1 chunk +0 lines, -5 lines 0 comments Download
M lib/common.js View 1 2 3 1 chunk +1 line, -2 lines 0 comments Download
M lib/content/elemHideEmulation.js View 1 2 3 4 2 chunks +5 lines, -2 lines 2 comments Download
M test/browser/elemHideEmulation.js View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 17
hub
2 years, 8 months ago (2017-06-09 17:08:44 UTC) #1
hub
I couldn't find an easy way to do the "require()" in elemHideEmulation. Which mean that ...
2 years, 8 months ago (2017-06-09 17:10:52 UTC) #2
kzar
Thanks for looking into this. On 2017/06/09 17:10:52, hub wrote: > I couldn't find an ...
2 years, 8 months ago (2017-06-12 12:49:38 UTC) #3
kzar
Sorry for the spam, I meant to publish these inline comments at the same time... ...
2 years, 8 months ago (2017-06-12 12:50:13 UTC) #4
hub
On 2017/06/12 12:49:38, kzar wrote: > Thanks for looking into this. > > On 2017/06/09 ...
2 years, 8 months ago (2017-06-12 13:10:16 UTC) #5
hub
https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js#newcode18 lib/content/elemHideEmulation.js:18: /* eslint-env browser */ On 2017/06/12 12:50:13, kzar wrote: ...
2 years, 8 months ago (2017-06-12 13:26:34 UTC) #6
kzar
On 2017/06/12 13:10:16, hub wrote: > On 2017/06/12 12:49:38, kzar wrote: > > Thanks for ...
2 years, 8 months ago (2017-06-12 13:32:55 UTC) #7
kzar
https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js#newcode18 lib/content/elemHideEmulation.js:18: /* eslint-env browser */ On 2017/06/12 13:26:33, hub wrote: ...
2 years, 8 months ago (2017-06-12 13:35:12 UTC) #8
hub
https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js#newcode18 lib/content/elemHideEmulation.js:18: /* eslint-env browser */ On 2017/06/12 13:35:12, kzar wrote: ...
2 years, 8 months ago (2017-06-12 14:03:14 UTC) #9
kzar
https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js#newcode18 lib/content/elemHideEmulation.js:18: /* eslint-env browser */ On 2017/06/12 14:03:14, hub wrote: ...
2 years, 8 months ago (2017-06-12 14:12:44 UTC) #10
hub
https://codereview.adblockplus.org/29460576/diff/29463567/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29463567/lib/content/elemHideEmulation.js#newcode154 lib/content/elemHideEmulation.js:154: console.error(new SyntaxError("Failed parsing ABP " + On 2017/06/12 14:12:44, ...
2 years, 8 months ago (2017-06-12 14:30:26 UTC) #11
hub
https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js#newcode18 lib/content/elemHideEmulation.js:18: /* eslint-env browser */ On 2017/06/12 14:12:44, kzar wrote: ...
2 years, 8 months ago (2017-06-12 15:21:50 UTC) #12
kzar
https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29460577/lib/content/elemHideEmulation.js#newcode457 lib/content/elemHideEmulation.js:457: if (typeof exports != "undefined") On 2017/06/12 14:03:13, hub ...
2 years, 7 months ago (2017-07-07 13:42:21 UTC) #13
hub
Current patch is based on master. Test fail because now that we are using require ...
2 years, 6 months ago (2017-08-10 14:46:22 UTC) #14
Wladimir Palant
Fairly straightforward change but this cannot land as long as tests aren't working. https://codereview.adblockplus.org/29460576/diff/29511631/lib/content/elemHideEmulation.js File ...
2 years, 6 months ago (2017-08-16 09:55:53 UTC) #15
kzar
https://codereview.adblockplus.org/29460576/diff/29511631/lib/content/elemHideEmulation.js File lib/content/elemHideEmulation.js (right): https://codereview.adblockplus.org/29460576/diff/29511631/lib/content/elemHideEmulation.js#newcode517 lib/content/elemHideEmulation.js:517: exports.splitSelector = splitSelector; On 2017/08/16 09:55:53, Wladimir Palant wrote: ...
2 years, 6 months ago (2017-08-16 09:59:05 UTC) #16
hub
2 years, 6 months ago (2017-08-16 16:10:57 UTC) #17
Closing as it is now part of https://codereview.adblockplus.org/29517687/
Sign in to reply to this message.

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