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

Issue 29958567: Issue 7104 - Clear style sheets when frame structure is updated (Closed)

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

Patch Set 1 #

Patch Set 2 : Use state object (includes rebase) #

Total comments: 9

Patch Set 3 : Use Object.create #

Patch Set 4 : Update comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -7 lines) Patch
M ext/background.js View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M lib/contentFiltering.js View 1 2 3 2 chunks +8 lines, -7 lines 0 comments Download

Messages

Total messages: 18
Manish Jethani
9 months, 2 weeks ago (2018-12-04 14:33:21 UTC) #1
kzar
I suspect this issue is a symptom of a problem in our page/frame logic, or ...
9 months, 2 weeks ago (2018-12-07 13:31:08 UTC) #2
kzar
On 2018/12/07 13:31:08, kzar wrote: > I suspect this issue is a symptom of a ...
9 months, 2 weeks ago (2018-12-07 13:35:40 UTC) #3
kzar
I wonder, is there a way to do this without putting content filtering logic in ...
9 months, 2 weeks ago (2018-12-07 14:08:51 UTC) #4
Manish Jethani
On 2018/12/07 13:35:40, kzar wrote: > On 2018/12/07 13:31:08, kzar wrote: > > I suspect ...
9 months, 2 weeks ago (2018-12-09 09:19:18 UTC) #5
Manish Jethani
On 2018/12/07 14:08:51, kzar wrote: > I wonder, is there a way to do this ...
9 months, 2 weeks ago (2018-12-09 09:45:21 UTC) #6
kzar
Hmm, perhaps we should just give frames a `state` Object which is cleared by `createFrame` ...
9 months, 1 week ago (2018-12-10 13:27:57 UTC) #7
Manish Jethani
On 2018/12/10 13:27:57, kzar wrote: > Hmm, perhaps we should just give frames a `state` ...
9 months, 1 week ago (2018-12-11 07:55:02 UTC) #8
kzar
On 2018/12/11 07:55:02, Manish Jethani wrote: > On 2018/12/10 13:27:57, kzar wrote: > > Hmm, ...
9 months ago (2018-12-18 11:38:35 UTC) #9
Manish Jethani
On 2018/12/18 11:38:35, kzar wrote: > On 2018/12/11 07:55:02, Manish Jethani wrote: > > On ...
9 months ago (2018-12-18 16:34:10 UTC) #10
kzar
https://codereview.adblockplus.org/29958567/diff/29964555/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29958567/diff/29964555/ext/background.js#newcode136 ext/background.js:136: frame.state = {}; Nit: How about `Object.create(null)`? https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js File ...
9 months ago (2018-12-19 10:22:01 UTC) #11
Manish Jethani
https://codereview.adblockplus.org/29958567/diff/29964555/ext/background.js File ext/background.js (right): https://codereview.adblockplus.org/29958567/diff/29964555/ext/background.js#newcode136 ext/background.js:136: frame.state = {}; On 2018/12/19 10:22:00, kzar wrote: > ...
9 months ago (2018-12-19 13:03:55 UTC) #12
kzar
https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js File lib/contentFiltering.js (left): https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js#oldcode116 lib/contentFiltering.js:116: // Ideally we would compare the old and new ...
9 months ago (2018-12-19 13:07:15 UTC) #13
Manish Jethani
https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js File lib/contentFiltering.js (left): https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js#oldcode116 lib/contentFiltering.js:116: // Ideally we would compare the old and new ...
9 months ago (2018-12-19 13:27:10 UTC) #14
Manish Jethani
https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js File lib/contentFiltering.js (left): https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js#oldcode116 lib/contentFiltering.js:116: // Ideally we would compare the old and new ...
9 months ago (2018-12-19 13:29:53 UTC) #15
kzar
https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js File lib/contentFiltering.js (left): https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js#oldcode116 lib/contentFiltering.js:116: // Ideally we would compare the old and new ...
9 months ago (2018-12-19 13:35:42 UTC) #16
Manish Jethani
Patch Set 4: Update comment Filed this: https://issues.adblockplus.org/ticket/7180 https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js File lib/contentFiltering.js (left): https://codereview.adblockplus.org/29958567/diff/29964555/lib/contentFiltering.js#oldcode116 lib/contentFiltering.js:116: // ...
9 months ago (2018-12-19 13:48:37 UTC) #17
kzar
9 months ago (2018-12-19 13:58:05 UTC) #18
Nice, thanks. LGTM
Sign in to reply to this message.

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