Issue 3915 - Properly handle special subscriptions

Issue 3915 - Properly handle special subscriptions

(Depends on the changes in https://codereview.adblockplus.org/29339651/ , dependencies file will also need to be updated once that lands.)

[kzar, 2016-04-12 12:22:02]

Patch Set 1

[Sebastian Noack, 2016-04-12 12:26:06]

https://codereview.adblockplus.org/29339654/diff/29339655/options.js
File options.js (right):

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode203
options.js:203: appendToListBox("excludedDomainsBox", RegExp.$1);
RegExp.$* is deprecated. However, since this is code is matter to be replaced
with the new options page anyway, I leave it up to you whether to address this.

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode477
options.js:477: if (subscription.special)
If we add this property it should be isSpecial.

[kzar, 2016-04-12 12:31:49]

https://codereview.adblockplus.org/29339654/diff/29339655/options.js
File options.js (right):

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode203
options.js:203: appendToListBox("excludedDomainsBox", RegExp.$1);
On 2016/04/12 12:26:06, Sebastian Noack wrote:
> RegExp.$* is deprecated. However, since this is code is matter to be replaced
> with the new options page anyway, I leave it up to you whether to address
this.

I would rather not mess with it. (Note this is not new code, I simply moved it
into a function.)

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode477
options.js:477: if (subscription.special)
On 2016/04/12 12:26:06, Sebastian Noack wrote:
> If we add this property it should be isSpecial.

I disagree as this way it's consistent with the `subscriptions.get` message. I
guess let's leave this up to Thomas to decide.

[Sebastian Noack, 2016-04-12 12:43:22]

https://codereview.adblockplus.org/29339654/diff/29339655/options.js
File options.js (right):

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode203
options.js:203: appendToListBox("excludedDomainsBox", RegExp.$1);
On 2016/04/12 12:31:49, kzar wrote:
> On 2016/04/12 12:26:06, Sebastian Noack wrote:
> > RegExp.$* is deprecated. However, since this is code is matter to be
replaced
> > with the new options page anyway, I leave it up to you whether to address
> this.
> 
> I would rather not mess with it. (Note this is not new code, I simply moved it
> into a function.)

I know that the logic here isn't new. But this would be a good opportunity to
clean it up. In particular since we have to test the whole options page anyway
after the recent changes. However, as this code is thrown away anyway when we
have the new options page, fine with me.

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode477
options.js:477: if (subscription.special)
On 2016/04/12 12:31:49, kzar wrote:
> On 2016/04/12 12:26:06, Sebastian Noack wrote:
> > If we add this property it should be isSpecial.
> 
> I disagree as this way it's consistent with the `subscriptions.get` message. I
> guess let's leave this up to Thomas to decide.

What does have the semantics of query parameters have to do with properties
describing an object? While speaking of consistency, also note that there is
already isDownloading.

[kzar, 2016-04-13 08:15:13]

https://codereview.adblockplus.org/29339654/diff/29339655/options.js
File options.js (right):

https://codereview.adblockplus.org/29339654/diff/29339655/options.js#newcode477
options.js:477: if (subscription.special)
On 2016/04/12 12:43:21, Sebastian Noack wrote:
> On 2016/04/12 12:31:49, kzar wrote:
> > On 2016/04/12 12:26:06, Sebastian Noack wrote:
> > > If we add this property it should be isSpecial.
> > 
> > I disagree as this way it's consistent with the `subscriptions.get` message.
I
> > guess let's leave this up to Thomas to decide.
> 
> What does have the semantics of query parameters have to do with properties
> describing an object? While speaking of consistency, also note that there is
> already isDownloading.

Fair enough, I renamed these in the new review.