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

Issue 29893559: Issue 6999 - Generate style sheets in background page (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year ago by Manish Jethani
Modified:
1 year ago
Reviewers:
Sebastian Noack
CC:
geo
Base URL:
https://hg.adblockplus.org/adblockpluschrome/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : Updated implementation #

Total comments: 6

Patch Set 3 : Rebase #

Patch Set 4 : Remove individual rule injection #

Patch Set 5 : Use generateStyleSheetForDomain from core #

Total comments: 3

Patch Set 6 : Update dependency, remove first argument to ElemHideEmulation constructor #

Patch Set 7 : Generate collapsing style sheet also in background page #

Patch Set 8 : Fix comment wrapping #

Total comments: 17

Patch Set 9 : Apply patch for ElementHidingTracer #

Patch Set 10 : Insert individual rules #

Total comments: 7

Patch Set 11 : Pass selectors if and only if trace is true #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -94 lines) Patch
M dependencies View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
M include.preload.js View 1 2 3 4 5 6 7 8 9 5 chunks +41 lines, -80 lines 0 comments Download
M lib/contentFiltering.js View 1 2 3 4 5 6 7 8 9 10 4 chunks +21 lines, -13 lines 0 comments Download

Messages

Total messages: 49
Manish Jethani
1 year ago (2018-09-27 05:35:20 UTC) #1
Manish Jethani
Patch Set 1 I'll have to test this out, but this is the idea.
1 year ago (2018-09-27 05:37:34 UTC) #2
Sebastian Noack
What is about the logic creating a shadow root (if supported), and creating the <style> ...
1 year ago (2018-09-27 15:26:04 UTC) #3
Manish Jethani
On 2018/09/27 15:26:04, Sebastian Noack wrote: > What is about the logic creating a shadow ...
1 year ago (2018-09-27 16:05:12 UTC) #4
Sebastian Noack
On 2018/09/27 16:05:12, Manish Jethani wrote: > On 2018/09/27 15:26:04, Sebastian Noack wrote: > > ...
1 year ago (2018-09-27 16:13:11 UTC) #5
geo
On 2018/09/27 16:13:11, Sebastian Noack wrote: > On 2018/09/27 16:05:12, Manish Jethani wrote: > > ...
1 year ago (2018-09-27 17:33:59 UTC) #6
Sebastian Noack
On 2018/09/27 17:33:59, geo wrote: > I've applied both patches, but after this one, element ...
1 year ago (2018-09-27 17:43:46 UTC) #7
geo
On 2018/09/27 17:43:46, Sebastian Noack wrote: > On 2018/09/27 17:33:59, geo wrote: > > I've ...
1 year ago (2018-09-27 18:24:01 UTC) #8
Sebastian Noack
Manish, just in case did you test it on older versions of Chrome and Firefox ...
1 year ago (2018-09-27 19:58:30 UTC) #9
Manish Jethani
On 2018/09/27 18:24:01, geo wrote: > On 2018/09/27 17:43:46, Sebastian Noack wrote: > > On ...
1 year ago (2018-09-27 21:30:40 UTC) #10
Manish Jethani
On 2018/09/27 21:30:40, Manish Jethani wrote: > On 2018/09/27 18:24:01, geo wrote: > > On ...
1 year ago (2018-09-27 21:32:08 UTC) #11
geo
On 2018/09/27 21:32:08, Manish Jethani wrote: > On 2018/09/27 21:30:40, Manish Jethani wrote: > > ...
1 year ago (2018-09-27 23:18:45 UTC) #12
Sebastian Noack
On 2018/09/27 23:18:45, geo wrote: > So, the error is just a string, " Error: ...
1 year ago (2018-09-27 23:34:51 UTC) #13
Manish Jethani
On 2018/09/27 23:34:51, Sebastian Noack wrote: > Manish, do you think in regard of #6957 ...
1 year ago (2018-09-28 07:13:28 UTC) #14
Manish Jethani
On 2018/09/28 07:13:28, Manish Jethani wrote: > On 2018/09/27 23:34:51, Sebastian Noack wrote: > > ...
1 year ago (2018-09-28 07:16:26 UTC) #15
Sebastian Noack
On 2018/09/28 07:16:26, Manish Jethani wrote: > So here's the plan: I'll test on the ...
1 year ago (2018-09-28 10:02:27 UTC) #16
Manish Jethani
On 2018/09/28 10:02:27, Sebastian Noack wrote: > On 2018/09/28 07:16:26, Manish Jethani wrote: > > ...
1 year ago (2018-09-28 10:07:52 UTC) #17
Manish Jethani
On 2018/09/28 10:07:52, Manish Jethani wrote: > On 2018/09/28 10:02:27, Sebastian Noack wrote: > > ...
1 year ago (2018-09-28 11:08:57 UTC) #18
Manish Jethani
Patch Set 2: Updated implementation This one is based on https://codereview.adblockplus.org/29894564/ We can refactor a ...
1 year ago (2018-09-28 13:25:58 UTC) #19
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js#newcode465 include.preload.js:465: for (let i = 0; i < selectors.length; i ...
1 year ago (2018-09-28 15:33:41 UTC) #20
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js#newcode465 include.preload.js:465: for (let i = 0; i < selectors.length; i ...
1 year ago (2018-09-28 16:04:44 UTC) #21
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js#newcode465 include.preload.js:465: for (let i = 0; i < selectors.length; i ...
1 year ago (2018-09-28 16:09:06 UTC) #22
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js#newcode465 include.preload.js:465: for (let i = 0; i < selectors.length; i ...
1 year ago (2018-09-28 16:12:51 UTC) #23
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js#newcode465 include.preload.js:465: for (let i = 0; i < selectors.length; i ...
1 year ago (2018-09-28 16:32:50 UTC) #24
Manish Jethani
Patch Set 3: Rebase Patch Set 4: Remove individual rule injection https://codereview.adblockplus.org/29893559/diff/29894581/include.preload.js File include.preload.js (right): ...
1 year ago (2018-09-28 20:20:23 UTC) #25
Manish Jethani
Patch Set 5: Use generateStyleSheetForDomain from core This one includes a dependency update. https://codereview.adblockplus.org/29893559/diff/29894607/lib/contentFiltering.js File ...
1 year ago (2018-09-28 20:57:01 UTC) #26
Sebastian Noack
I still kinda feel it would be simpler if we message the background page (regardless ...
1 year ago (2018-09-29 00:09:08 UTC) #27
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29894607/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29894607/include.preload.js#oldcode403 include.preload.js:403: () => {}, This argument should be removed now, ...
1 year ago (2018-09-29 02:07:21 UTC) #28
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29894607/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29894607/include.preload.js#oldcode403 include.preload.js:403: () => {}, On 2018/09/29 02:07:20, Sebastian Noack wrote: ...
1 year ago (2018-09-29 02:20:47 UTC) #29
Manish Jethani
Patch Set 6: Update dependency, remove first argument to ElemHideEmulation constructor Patch Set 7: Generate ...
1 year ago (2018-09-29 10:41:39 UTC) #30
Manish Jethani
Patch Set 8: Fix comment wrapping
1 year ago (2018-09-29 10:43:59 UTC) #31
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#oldcode401 include.preload.js:401: Nit: The blank line here no longer seems to ...
1 year ago (2018-09-29 14:36:13 UTC) #32
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#oldcode489 include.preload.js:489: this.tracer.addSelectors(selectors, filters); On 2018/09/29 14:36:13, Sebastian Noack wrote: > ...
1 year ago (2018-09-29 15:58:49 UTC) #33
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#oldcode489 include.preload.js:489: this.tracer.addSelectors(selectors, filters); On 2018/09/29 15:58:48, Manish Jethani wrote: > ...
1 year ago (2018-09-29 16:39:31 UTC) #34
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#oldcode489 include.preload.js:489: this.tracer.addSelectors(selectors, filters); On 2018/09/29 16:39:30, Sebastian Noack wrote: > ...
1 year ago (2018-09-29 17:28:05 UTC) #35
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#oldcode489 include.preload.js:489: this.tracer.addSelectors(selectors, filters); On 2018/09/29 17:28:04, Manish Jethani wrote: > ...
1 year ago (2018-09-29 21:01:50 UTC) #36
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#newcode428 include.preload.js:428: style.textContent += styleSheet; On 2018/09/29 21:01:49, Sebastian Noack wrote: ...
1 year ago (2018-09-30 08:06:21 UTC) #37
Manish Jethani
Patch Set 9: Apply patch for ElementHidingTracer https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#oldcode489 include.preload.js:489: this.tracer.addSelectors(selectors, filters); ...
1 year ago (2018-09-30 08:36:38 UTC) #38
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#newcode428 include.preload.js:428: style.textContent += styleSheet; On 2018/09/30 08:06:21, Manish Jethani wrote: ...
1 year ago (2018-09-30 14:19:24 UTC) #39
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#newcode428 include.preload.js:428: style.textContent += styleSheet; On 2018/09/30 14:19:24, Sebastian Noack wrote: ...
1 year ago (2018-10-01 14:01:37 UTC) #40
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#newcode428 include.preload.js:428: style.textContent += styleSheet; On 2018/10/01 14:01:36, Manish Jethani wrote: ...
1 year ago (2018-10-01 14:24:02 UTC) #41
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js File include.preload.js (right): https://codereview.adblockplus.org/29893559/diff/29895563/include.preload.js#newcode428 include.preload.js:428: style.textContent += styleSheet; On 2018/10/01 14:24:02, Manish Jethani wrote: ...
1 year ago (2018-10-01 19:42:27 UTC) #42
Manish Jethani
Patch Set 10: Insert individual rules
1 year ago (2018-10-02 15:43:28 UTC) #43
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29898573/include.preload.js File include.preload.js (left): https://codereview.adblockplus.org/29893559/diff/29898573/include.preload.js#oldcode401 include.preload.js:401: Nit: Not sure if this blank line still helps ...
1 year ago (2018-10-02 17:14:03 UTC) #44
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js File lib/contentFiltering.js (right): https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js#newcode249 lib/contentFiltering.js:249: response.rules = [...rulesFromStyleSheet(styleSheet.code)]; On 2018/10/02 17:14:02, Sebastian Noack wrote: ...
1 year ago (2018-10-02 17:15:38 UTC) #45
Manish Jethani
https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js File lib/contentFiltering.js (right): https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js#newcode249 lib/contentFiltering.js:249: response.rules = [...rulesFromStyleSheet(styleSheet.code)]; On 2018/10/02 17:15:38, Sebastian Noack wrote: ...
1 year ago (2018-10-02 17:23:25 UTC) #46
Sebastian Noack
https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js File lib/contentFiltering.js (right): https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js#newcode249 lib/contentFiltering.js:249: response.rules = [...rulesFromStyleSheet(styleSheet.code)]; On 2018/10/02 17:23:24, Manish Jethani wrote: ...
1 year ago (2018-10-02 17:28:22 UTC) #47
Manish Jethani
Patch Set 11: Pass selectors if and only if trace is true https://codereview.adblockplus.org/29893559/diff/29898573/lib/contentFiltering.js File lib/contentFiltering.js ...
1 year ago (2018-10-02 19:07:47 UTC) #48
Sebastian Noack
1 year ago (2018-10-02 19:39:07 UTC) #49
LGTM
Sign in to reply to this message.

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