Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Background Sync should be disabled by default #4709

Closed
pes10k opened this issue Jun 4, 2019 · 6 comments · Fixed by brave/brave-core#2802 or brave/brave-core#3553
Closed

Background Sync should be disabled by default #4709

pes10k opened this issue Jun 4, 2019 · 6 comments · Fixed by brave/brave-core#2802 or brave/brave-core#3553
Assignees
@fmarier
Labels
priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes regression release-notes/include security
Milestone
1.1.x - Release

Comments

@pes10k
Copy link
Contributor

pes10k commented Jun 4, 2019 •
edited by bsclifton
Loading

Test plan

See brave/brave-core#2802

Description

Looks like Background Sync snuck back into being enabled by default.

https://deanhume.github.io/Service-Workers-BackgroundSync/ gives an example / demo of the functionality working

and it appears as enabled by default in permissions / settings (Version 0.66.79 Chromium: 75.0.3770.38 (Official Build) beta (64-bit))
screen_shot_2019-06-03_at_5 01 58_pm

Expected behavior
Background Sync should be disabled by default

Possible regressions of:

Possible duplicate of:
@bsclifton
Copy link
Member

cc: @rebron

@bsclifton
Copy link
Member

@snyderp would your recommendation be to have this move to Ask or Block by default?

@pes10k
Copy link
Contributor Author

pes10k commented Jun 10, 2019

I thinks users will have no idea at all what it does, so I suggest Block by default

@diracdeltas diracdeltas added priority/P3 The next thing for us to work on. It'll ride the trains. regression labels Jun 18, 2019
@fmarier
Copy link
Member

fmarier commented Jun 25, 2019

Looks pretty trivial: https://github.com/bromite/bromite/blob/master/build/patches/disable-background-sync-by-default.patch

@fmarier fmarier self-assigned this Jun 25, 2019
fmarier added a commit to brave/brave-core that referenced this issue Jun 25, 2019
@fmarier
Disable background-sync by default (fixes brave/brave-browser#4709)
05bf5d7
@fmarier fmarier mentioned this issue Jun 25, 2019
Merged
33 tasks
@fmarier fmarier added the QA/Yes label Jun 25, 2019
@bsclifton bsclifton added this to the 0.69.x - Nightly milestone Jun 26, 2019
@bsclifton bsclifton removed this from the 0.69.x - Beta milestone Sep 3, 2019
fmarier added a commit to brave/brave-core that referenced this issue Sep 12, 2019
@fmarier
Disable background-sync by default (fixes brave/brave-browser#4709)
41a7077
fmarier added a commit to brave/brave-core that referenced this issue Sep 13, 2019
@fmarier
Disable background-sync by default (fixes brave/brave-browser#4709)
85d6cc0
fmarier added a commit to brave/brave-core that referenced this issue Sep 18, 2019
@fmarier
Disable background-sync by default (fixes brave/brave-browser#4709)
cf04867
@fmarier fmarier added this to the 0.72.x - Nightly milestone Sep 23, 2019
fmarier added a commit to brave/brave-core that referenced this issue Sep 24, 2019
@fmarier
Revert "Disable background-sync by default (fixes brave/brave-browser…
c8f2e60 
…#4709)"

This reverts commit cf04867.
@bsclifton bsclifton reopened this Sep 24, 2019
@bsclifton bsclifton removed this from the 0.72.x - Nightly milestone Sep 24, 2019
@bsclifton
Copy link
Member

Re-opening after brave/brave-core#2802 was reverted with brave/brave-core#3517

@fmarier fmarier mentioned this issue Sep 26, 2019
Merged
32 tasks
fmarier added a commit to brave/brave-core that referenced this issue Sep 30, 2019
@fmarier
Disable background-sync by default (fixes brave/brave-browser#4709)
a8d2c71
@fmarier fmarier added this to the 0.72.x - Nightly milestone Oct 1, 2019
@LaurenWags
Copy link
Member

LaurenWags commented Oct 28, 2019 •
edited by GeetaSarvadnya
Loading

Verified passed with

Brave 0.72.110 Chromium: 78.0.3904.70 (Official Build) dev (64-bit)
Revision edb9c9f3de0247fd912a77b7f6cae7447f6d3ad5-refs/branch-heads/3904@{#800}
OS macOS Version 10.13.6 (Build 17G5019)

4709

both

Verification passed on

Brave 0.72.110 Chromium: 78.0.3904.70 (Official Build) dev (64-bit)
Revision edb9c9f3de0247fd912a77b7f6cae7447f6d3ad5-refs/branch-heads/3904@{#800}
OS Ubuntu 18.04 LTS

Verified test plan from brave/brave-core#2802
image
image

Verified that Motion Sensors are off by default on clean profile and upgrade from 0.70.x
image

Verification passed on

Brave 1.1.1 Chromium: 78.0.3904.97 (Official Build) beta (64-bit)
Revision 021b9028c246d820be17a10e5b393ee90f41375e-refs/branch-heads/3904@{#859}
OS Windows 10 OS Version 1803 (Build 17134.1006)

image

@LaurenWags LaurenWags mentioned this issue Dec 9, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fmarier fmarier

Labels
priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes regression release-notes/include security
Projects
None yet
Milestone
1.1.x - Release
7 participants
@pes10k @fmarier @diracdeltas @bsclifton @LaurenWags @btlechowski @GeetaSarvadnya