Issue 5925 - Update adblockplusui dependency to update strings and use checkboxes for Acceptable Ads

After this change will land Tamara will start updating the missing translations in the crowdin.

Currently German is only 6% is translated in the crowdin(New options page), Tamara will try to update the translations as much as possible for the new options page, she planed/supposedly to/will finish updating the translations in the crowdin until the end of this week.

[saroyanm, 2017-10-24 12:15:39]

@Sebastian can you have a look please.

I think would be great to push the strings in order to fix the translations
until the launch @Tamara will take care of it.

[saroyanm, 2017-10-24 15:39:03]

@kzar I did notice that Sebastian is Out Of Office for Chrome Dev Summit, so
maybe you can have a look and approve this changes if you agree that we should
push this changes during the string freeze, considering the fact that Tamara
will update translations in the Crowdin before the release.

See relevant discussion here ->
https://issues.adblockplus.org/ticket/5874#comment:9

[kzar, 2017-10-24 15:49:11]

Please could you open an issue for this dependency update Manvel, mention which
(relevant) incidental changes would be included by the update (and what
functionality that might effect) as well? Otherwise they'll be no way for the QA
guys to test for regressions before the release.

On 2017/10/24 15:39:03, saroyanm wrote:
> @kzar I did notice that Sebastian is Out Of Office for Chrome Dev Summit, so
> maybe you can have a look and approve this changes if you agree that we should
> push this changes during the string freeze, considering the fact that Tamara
> will update translations in the Crowdin before the release.
> 
> See relevant discussion here ->
> https://issues.adblockplus.org/ticket/5874#comment:9

Well in the discussion linked there you mention "Unless we are not planing to
launch this week. Otherwise I'll have to hold this back.", but I've not been
involved with planning for the launch at all so I'm not sure. Do you have any
idea for when the launch is planned?

[saroyanm, 2017-10-24 16:17:55]

On 2017/10/24 15:49:11, kzar wrote:
> Please could you open an issue for this dependency update Manvel, mention
which
> (relevant) incidental changes would be included by the update (and what
> functionality that might effect) as well? Otherwise they'll be no way for the
QA
> guys to test for regressions before the release.
Great idea, Done. #5925
> On 2017/10/24 15:39:03, saroyanm wrote:
> > @kzar I did notice that Sebastian is Out Of Office for Chrome Dev Summit, so
> > maybe you can have a look and approve this changes if you agree that we
should
> > push this changes during the string freeze, considering the fact that Tamara
> > will update translations in the Crowdin before the release.
> > 
> > See relevant discussion here ->
> > https://issues.adblockplus.org/ticket/5874#comment:9
> 
> Well in the discussion linked there you mention "Unless we are not planing to
> launch this week. Otherwise I'll have to hold this back.", but I've not been
> involved with planning for the launch at all so I'm not sure. Do you have any
> idea for when the launch is planned?
I'm not aware of the fixed date (Added note in the ticket). Maybe we can wait
until tomorrow, to be sure if there is one.

[kzar, 2017-10-24 16:54:13]

On 2017/10/24 16:17:55, saroyanm wrote:
> I'm not aware of the fixed date (Added note in the ticket). 
> Maybe we can wait until tomorrow, to be sure if there is one.

Yea, I think that's a good idea. I've updated the issue in the mean time so
hopefully we can push this first thing tomorrow if Sebastian etc are OK with it.

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies#newcode5
dependencies:5: adblockplusui = adblockplusui hg:a65aba31839f git:a315379
I updated the issue, perhaps we should update it to hg:e7371d9c682d git:99d9004
in order to include Wlaidmir's recent issue reporter improvements?

[saroyanm, 2017-10-24 17:04:49]

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies#newcode5
dependencies:5: adblockplusui = adblockplusui hg:a65aba31839f git:a315379
On 2017/10/24 16:54:13, kzar wrote:
> I updated the issue, perhaps we should update it to hg:e7371d9c682d
git:99d9004
> in order to include Wlaidmir's recent issue reporter improvements?

I think there might more coming till tomorrow.
Let's update the dependency tomorrow.

We will have our regular weekly ABPUI meeting tomorrow 12:00CEST (I'll ask
Winsley to add you as an optional participant), Winsley who manages the
project(New options page) will be there as well(he is currently on vacation),
I'll update the issue or ask him update the issue accordingly after the meeting
tomorrow.

[saroyanm, 2017-10-24 17:13:15]

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies#newcode5
dependencies:5: adblockplusui = adblockplusui hg:a65aba31839f git:a315379
On 2017/10/24 17:04:49, saroyanm wrote:
> On 2017/10/24 16:54:13, kzar wrote:
> > I updated the issue, perhaps we should update it to hg:e7371d9c682d
> git:99d9004
> > in order to include Wlaidmir's recent issue reporter improvements?
> 
> I think there might more coming till tomorrow.
> Let's update the dependency tomorrow.
> 
> We will have our regular weekly ABPUI meeting tomorrow 12:00CEST (I'll ask
> Winsley to add you as an optional participant), Winsley who manages the
> project(New options page) will be there as well(he is currently on vacation),
> I'll update the issue or ask him update the issue accordingly after the
meeting
> tomorrow.

Anyway I've updated the dependencies to keep in sync with the ticket
description.

[saroyanm, 2017-10-24 17:15:43]

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29587623/diff/29587624/dependencies#newcode5
dependencies:5: adblockplusui = adblockplusui hg:a65aba31839f git:a315379
On 2017/10/24 17:13:15, saroyanm wrote:
> On 2017/10/24 17:04:49, saroyanm wrote:
> > On 2017/10/24 16:54:13, kzar wrote:
> > > I updated the issue, perhaps we should update it to hg:e7371d9c682d
> > git:99d9004
> > > in order to include Wlaidmir's recent issue reporter improvements?
> > 
> > I think there might more coming till tomorrow.
> > Let's update the dependency tomorrow.
> > 
> > We will have our regular weekly ABPUI meeting tomorrow 12:00CEST (I'll ask
> > Winsley to add you as an optional participant), Winsley who manages the
> > project(New options page) will be there as well(he is currently on
vacation),
> > I'll update the issue or ask him update the issue accordingly after the
> meeting
> > tomorrow.
> 
> Anyway I've updated the dependencies to keep in sync with the ticket
> description.

Scratch that: the meeting is moved to the Thursday, so yes if we will have the
Go by tomorrow I'll push this changes.

[kzar, 2017-10-24 17:34:07]

On 2017/10/24 17:15:43, saroyanm wrote:
> Scratch that: the meeting is moved to the Thursday, so yes if we will have the
> Go by tomorrow I'll push this changes.

Cool, let's see what Wladimir / Sebastian say about pushing this. But the change
itself LGTM.

[saroyanm, 2017-10-24 17:34:59]

On 2017/10/24 17:34:07, kzar wrote:
> On 2017/10/24 17:15:43, saroyanm wrote:
> > Scratch that: the meeting is moved to the Thursday, so yes if we will have
the
> > Go by tomorrow I'll push this changes.
> 
> Cool, let's see what Wladimir / Sebastian
Agree.

[Wladimir Palant, 2017-10-26 10:10:17]

NOT LGTM

This will break issue reporter. Please have a look at the integration notes in
https://issues.adblockplus.org/ticket/5887 and
https://issues.adblockplus.org/ticket/5888.

[saroyanm, 2017-10-26 12:56:48]

On 2017/10/26 10:10:17, Wladimir Palant wrote:
> NOT LGTM
> 
> This will break issue reporter. Please have a look at the integration notes in
> https://issues.adblockplus.org/ticket/5887 and
> https://issues.adblockplus.org/ticket/5888.

Why are we importing issue reporter in metadata.chrome, but not into
metadata.gecko ?
Wasn't the plan to first release it for the Firefox than move to Chrome and Edge
?

[kzar, 2017-10-26 13:11:36]

On 2017/10/26 12:56:48, saroyanm wrote:
> Why are we importing issue reporter in metadata.chrome, but not into
> metadata.gecko ?
> Wasn't the plan to first release it for the Firefox than move to Chrome and
Edge
> ?

metadata.chrome applies to all builds, so by adding the issue reporter entries
to metadata.chrome it should also add it for the gecko Firefox build.

As for the releases, I'm not sure the exact plan but my understanding was we'd
first release the new options page for Firefox, but later for Chrome. I didn't
realise that applied to the issue reporter too. Additionally we could simply do
a Firefox release and then wait a while before performing a Chrome release if
we're concerned about it (I'm not FWIW).

[saroyanm, 2017-10-26 13:42:06]

On 2017/10/26 13:11:36, kzar wrote:
> On 2017/10/26 12:56:48, saroyanm wrote:
> > Why are we importing issue reporter in metadata.chrome, but not into
> > metadata.gecko ?
> > Wasn't the plan to first release it for the Firefox than move to Chrome and
> Edge
> > ?
> 
> metadata.chrome applies to all builds, so by adding the issue reporter entries
> to metadata.chrome it should also add it for the gecko Firefox build.
> 
> As for the releases, I'm not sure the exact plan but my understanding was we'd
> first release the new options page for Firefox, but later for Chrome. I didn't
> realise that applied to the issue reporter too. Additionally we could simply
do
> a Firefox release and then wait a while before performing a Chrome release if
> we're concerned about it (I'm not FWIW).

The only concern I see, that it might not be tested in the edge for example and
it may not function as expected if we have emergency release in edge. But I'm
not sure if it was considered during development and if it does have
compatibility issues, if doesn't we should be fine, or we should prioritize
testing that assumption after current release.

My assumption is that the reason of it being in the Chrome release is because of
"popup" is implemented in the adblockpluschrome and it's easier to release for
all platform rather than having conditions in "popup" code and manage that
conditions separately than "metadata" files.

[kzar, 2017-10-26 13:47:40]

On 2017/10/26 13:42:06, saroyanm wrote:
> The only concern I see, that it might not be tested in the edge for example
and
> it may not function as expected if we have emergency release in edge.

Edge releases still come from the edge branch unless I'm mistaken, so I don't
think that is a pressing concern.

IMO just go ahead and add these entries to metadata.chrome, that would pass
review with me if it's done right.

[saroyanm, 2017-10-26 14:09:18]

New patch uploaded.

[kzar, 2017-10-26 14:38:16]

LGTM, you can go ahead and push this once you're ready.

[Wladimir Palant, 2017-10-26 14:40:45]

LGTM

[Wladimir Palant, 2017-10-26 14:44:30]

For reference, I'm not aware of any plans to land issue reporter for Firefox
only, and I don't know why we should.

[saroyanm, 2017-10-26 14:55:29]

On 2017/10/26 14:44:30, Wladimir Palant wrote:
> For reference, I'm not aware of any plans to land issue reporter for Firefox
> only, and I don't know why we should.

Noted.