Issue 4644 - Pass search messages to our devtools panel

Issue 4644 - Pass search messages to our devtools panel

[kzar, 2016-11-14 19:57:34]

Patch Set 1

[Sebastian Noack, 2016-11-17 15:34:34]

Well, there is not much of a point in landing this until the corresponding
functionality has been implemented in adblockplusui, and the dependency has been
updated.

[kzar, 2016-11-17 15:38:39]

True of course.

[kzar, 2016-11-30 15:58:55]

On 2016/11/17 15:34:34, Sebastian Noack wrote:
> Well, there is not much of a point in landing this until the corresponding
> functionality has been implemented in adblockplusui, and the dependency has
been
> updated.

(The corresponding functionality has now been pushed to adblockplusui.)

[Sebastian Noack, 2017-01-12 23:25:02]

Besides that, I think this should be committed together with the dependency
update.

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js
File chrome/devtools.js (right):

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:20: var panelWindow = null;
Is the panel (and it's window) a singleton? What if you use the developer tools
panel on multiple pages simultaneously?

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:20: var panelWindow = null;
Any reason to not use let (instead of var) here?

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:35: function (panel)
I wonder whether we should use arrow functions here? Also I wonder what the rule
for using arrow functions in general should be? It seems in recent code (that
you wrote or reviewed) we use arrow functions for all anonymous function unless
we explicitly need no implicit binding.

[kzar, 2017-01-13 08:30:08]

Patch Set 2 : Rebased
Patch Set 3 : Use let and arrow functions

> Besides that, I think this should be committed together with 
> the dependency update.

There's no dependency update required now since the adblockplusui dependency has
already been updated for other reasons.

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js
File chrome/devtools.js (right):

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:20: var panelWindow = null;
On 2017/01/12 23:25:02, Sebastian Noack wrote:
> Is the panel (and it's window) a singleton? What if you use the developer
tools
> panel on multiple pages simultaneously?

Good point, but I just tested and it works fine with multiple windows. I think
devtools.js is run in it's own context for each developer tools window.

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:20: var panelWindow = null;
On 2017/01/12 23:25:02, Sebastian Noack wrote:
> Any reason to not use let (instead of var) here?

Done.

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:35: function (panel)
On 2017/01/12 23:25:02, Sebastian Noack wrote:
> I wonder whether we should use arrow functions here? Also I wonder what the
rule
> for using arrow functions in general should be? It seems in recent code (that
> you wrote or reviewed) we use arrow functions for all anonymous function
unless
> we explicitly need no implicit binding.

I think the rule is to use let and arrow functions wherever possible now. I
should really get around to updating the existing code too, I'll try and open a
review for that today.

[Sebastian Noack, 2017-01-13 11:57:38]

LGTM

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js
File chrome/devtools.js (right):

https://codereview.adblockplus.org/29362526/diff/29362527/chrome/devtools.js#...
chrome/devtools.js:35: function (panel)
On 2017/01/13 08:30:08, kzar wrote:
> On 2017/01/12 23:25:02, Sebastian Noack wrote:
> > I wonder whether we should use arrow functions here? Also I wonder what the
> rule
> > for using arrow functions in general should be? It seems in recent code
(that
> > you wrote or reviewed) we use arrow functions for all anonymous function
> unless
> > we explicitly need no implicit binding.
> 
> I think the rule is to use let and arrow functions wherever possible now. I
> should really get around to updating the existing code too, I'll try and open
a
> review for that today.

I tend to agree. Mind filing a review to add it to
https://adblockplus.org/coding-style#javascript ?

[kzar, 2017-01-16 07:09:40]

On 2017/01/13 11:57:38, Sebastian Noack wrote:
> I tend to agree. Mind filing a review to add it to
> https://adblockplus.org/coding-style#javascript ?

Done https://codereview.adblockplus.org/29371951/