Issue 29325845: Restrict browser promotion to Firefox and Chrome (Closed)
|
Created:
Sept. 3, 2015, 3:53 p.m. by Felix Dahlke Modified:
Sept. 3, 2015, 6:15 p.m. CC:
mathias, kirill Visibility:
Public. |
DescriptionRestrict browser promotion to Firefox and Chrome
Patch Set 1 #
Total comments: 3
MessagesTotal messages: 7
addonName=adblockplus has a few clients we should IMHO not bother with this, such as Thunderbird, and addonName=adblockpluschrome will, ironically, show up for Adblock Browser on iOS.
https://codereview.adblockplus.org/29325845/diff/29325846/2 File 2 (right): https://codereview.adblockplus.org/29325845/diff/29325846/2#newcode3 2:3: target = extension=adblockpluschrome application=chrome So we don't want to show the notification on Opera and Safari? Please explain.
https://codereview.adblockplus.org/29325845/diff/29325846/2 File 2 (right): https://codereview.adblockplus.org/29325845/diff/29325846/2#newcode3 2:3: target = extension=adblockpluschrome application=chrome On 2015/09/03 16:10:29, Sebastian Noack wrote: > So we don't want to show the notification on Opera and Safari? Please explain. Doesn't seem worth the trouble, we didn't test this, I'm not even sure we have an opt-out in Safari. Note that extension=adblockpluschrome should already achieve that, this patch shouldn't change anything about that.
LGTM https://codereview.adblockplus.org/29325845/diff/29325846/2 File 2 (right): https://codereview.adblockplus.org/29325845/diff/29325846/2#newcode3 2:3: target = extension=adblockpluschrome application=chrome On 2015/09/03 16:11:59, Felix Dahlke wrote: > On 2015/09/03 16:10:29, Sebastian Noack wrote: > > So we don't want to show the notification on Opera and Safari? Please explain. > > Doesn't seem worth the trouble, we didn't test this, I'm not even sure we have > an opt-out in Safari. Note that extension=adblockpluschrome should already > achieve that, this patch shouldn't change anything about that. Yes, we have the opt-out on Safari. The UI is the same as on Chrome. But you are right, the extension name already excludes Opera and Safari anyway. It's a shame though, as presumably Safari and theirfore OS X users often have iPhones as well. But if we didn't test that yet, it's too late now. :(
On 2015/09/03 16:15:36, Sebastian Noack wrote: > LGTM > > https://codereview.adblockplus.org/29325845/diff/29325846/2 > File 2 (right): > > https://codereview.adblockplus.org/29325845/diff/29325846/2#newcode3 > 2:3: target = extension=adblockpluschrome application=chrome > On 2015/09/03 16:11:59, Felix Dahlke wrote: > > On 2015/09/03 16:10:29, Sebastian Noack wrote: > > > So we don't want to show the notification on Opera and Safari? Please > explain. > > > > Doesn't seem worth the trouble, we didn't test this, I'm not even sure we have > > an opt-out in Safari. Note that extension=adblockpluschrome should already > > achieve that, this patch shouldn't change anything about that. > > Yes, we have the opt-out on Safari. The UI is the same as on Chrome. But you are > right, the extension name already excludes Opera and Safari anyway. It's a shame > though, as presumably Safari and theirfore OS X users often have iPhones as > well. But if we didn't test that yet, it's too late now. :( We could still include them in the final notification - the test is for which notification mechanism/variant works best.
LGTM
|
