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

Issue 29564767: Issue 242 - Use user style sheets on Chromium (Closed)

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

Description

Issue 242 - Use user style sheets on Chromium

Patch Set 1 #

Patch Set 2 : Rebase and update target version #

Patch Set 3 : Rewrite #

Total comments: 13

Patch Set 4 : Check for Gecko #

Patch Set 5 : Use feature detection for cssOrigin #

Patch Set 6 : Update comment #

Patch Set 7 : Rebase #

Total comments: 6

Patch Set 8 : Fix regex matching and check only for Gecko #

Patch Set 9 : Do not check error message #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -14 lines) Patch
M include.preload.js View 1 2 3 chunks +3 lines, -1 line 0 comments Download
M lib/cssInjection.js View 1 2 3 4 5 6 7 8 4 chunks +43 lines, -13 lines 0 comments Download

Messages

Total messages: 27
Manish Jethani
2 years, 1 month ago (2017-10-04 22:32:11 UTC) #1
Manish Jethani
Patch Set 1 If we assume that Chromium version 60+ will support user style sheets ...
2 years, 1 month ago (2017-10-04 22:37:40 UTC) #2
Sebastian Noack
LGTM. But before landing this, can we at least wait until the fix for https://crbug.com/632009 ...
2 years, 1 month ago (2017-10-04 22:43:01 UTC) #3
Manish Jethani
On 2017/10/04 22:43:01, Sebastian Noack wrote: > LGTM. Thanks. > But before landing this, can ...
2 years, 1 month ago (2017-10-04 23:11:36 UTC) #4
Manish Jethani
Patch Set 2: Rebase and update target version User style sheet support is already in ...
2 years ago (2017-10-20 00:42:50 UTC) #5
Sebastian Noack
I wonder whether we should better wait until we can remove the stylesheet as well. ...
2 years ago (2017-10-20 01:29:58 UTC) #6
Manish Jethani
On 2017/10/20 01:29:58, Sebastian Noack wrote: > I wonder whether we should better wait until ...
2 years ago (2017-10-20 01:54:51 UTC) #7
Sebastian Noack
On 2017/10/20 01:54:51, Manish Jethani wrote: > On 2017/10/20 01:29:58, Sebastian Noack wrote: > > ...
2 years ago (2017-10-20 02:03:16 UTC) #8
Manish Jethani
On 2017/10/20 01:54:51, Manish Jethani wrote: > [...] > it may be a while before ...
2 years ago (2017-10-27 12:09:03 UTC) #9
Manish Jethani
I have rewritten this now based on master. I think we should start using tabs.insertCSS ...
1 year, 9 months ago (2018-01-31 08:24:55 UTC) #10
Manish Jethani
On 2018/01/31 08:24:55, Manish Jethani wrote: > I have rewritten this now based on master. ...
1 year, 9 months ago (2018-01-31 08:25:31 UTC) #11
kzar
https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js#newcode36 lib/cssInjection.js:36: info.platform != "chromium"; IMO we only know removeCSS works ...
1 year, 9 months ago (2018-01-31 11:11:32 UTC) #12
Sebastian Noack
https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js#newcode34 lib/cssInjection.js:34: parseInt(info.platformVersion, 10) >= 66); Perhaps, we should consider going ...
1 year, 9 months ago (2018-01-31 11:23:37 UTC) #13
kzar
https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js#newcode36 lib/cssInjection.js:36: info.platform != "chromium"; On 2018/01/31 11:23:37, Sebastian Noack wrote: ...
1 year, 9 months ago (2018-01-31 11:25:29 UTC) #14
Sebastian Noack
https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js#newcode36 lib/cssInjection.js:36: info.platform != "chromium"; On 2018/01/31 11:25:29, kzar wrote: > ...
1 year, 9 months ago (2018-01-31 11:54:44 UTC) #15
kzar
https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js#newcode36 lib/cssInjection.js:36: info.platform != "chromium"; On 2018/01/31 11:54:44, Sebastian Noack wrote: ...
1 year, 9 months ago (2018-01-31 11:57:56 UTC) #16
Manish Jethani
Patch Set 4: Check for Gecko Comments inline. https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685558/lib/cssInjection.js#newcode34 lib/cssInjection.js:34: parseInt(info.platformVersion, ...
1 year, 9 months ago (2018-01-31 12:36:27 UTC) #17
Manish Jethani
Patch Set 5: Use feature detection for cssOrigin
1 year, 9 months ago (2018-01-31 13:10:45 UTC) #18
Manish Jethani
Patch Set 6: Update comment
1 year, 9 months ago (2018-01-31 13:13:56 UTC) #19
Manish Jethani
Patch Set 7: Rebase
1 year, 9 months ago (2018-01-31 13:18:38 UTC) #20
Sebastian Noack
https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js#newcode36 lib/cssInjection.js:36: info.platform == "gecko"; tabs.removeCSS() was added in Firefox 49. ...
1 year, 9 months ago (2018-01-31 14:30:06 UTC) #21
Manish Jethani
Patch Set 8: Fix regex matching and check only for Gecko https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js File lib/cssInjection.js (right): ...
1 year, 9 months ago (2018-02-01 08:05:08 UTC) #22
Sebastian Noack
https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js#newcode84 lib/cssInjection.js:84: if (/\bError processing cssOrigin\b/.test(error.message) == -1) On 2018/02/01 08:05:07, ...
1 year, 9 months ago (2018-02-01 10:32:50 UTC) #23
Manish Jethani
Patch Set 9: Do not check error message https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js File lib/cssInjection.js (right): https://codereview.adblockplus.org/29564767/diff/29685587/lib/cssInjection.js#newcode84 lib/cssInjection.js:84: if ...
1 year, 9 months ago (2018-02-01 10:40:38 UTC) #24
Sebastian Noack
LGTM
1 year, 9 months ago (2018-02-01 11:03:23 UTC) #25
Manish Jethani
On 2018/02/01 11:03:23, Sebastian Noack wrote: > LGTM Thanks! Dave, would you like to take ...
1 year, 9 months ago (2018-02-01 11:08:40 UTC) #26
kzar
1 year, 9 months ago (2018-02-01 11:21:52 UTC) #27
On 2018/02/01 11:08:40, Manish Jethani wrote:
> On 2018/02/01 11:03:23, Sebastian Noack wrote:
> > LGTM
> 
> Thanks!
> 
> Dave, would you like to take a look?

No, you go ahead if Sebastian's happy that's good enough for me.
Sign in to reply to this message.

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