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.

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

Change the HTTP error code Brave uses when blocking a network request to better match what other browsers and tools expect, to increase compatibility with crowdsourced filter lists #10063

Closed
pes10k opened this issue Jun 3, 2020 · 6 comments · Fixed by brave/brave-core#6632 or brave/brave-core#7030

Comments

@pes10k
Copy link
Contributor

pes10k commented Jun 3, 2020

A long time ago we started "lying" to pages about scripts being blocked; even when we blocked a script, we didn't throw onerror, we treated it as a 200 empty.

Now that we (all but tiny details) match uBO in capabilities, we should do what uBO does and trigger error path when adblock-rust blocks something

We should continue reporting 200 when we block-but-replace

@pes10k pes10k added the privacy-pod Feature work for the Privacy & Web Compatibility pod label Jun 3, 2020
@bsclifton bsclifton added this to the 1.18.x - Nightly milestone Nov 2, 2020
@bsclifton
Copy link
Member

bsclifton commented Nov 2, 2020

@pes10k does this need QA? If so - can you add steps (and then mark as QA/yes). Also not sure if there's a more concise title we can use for the release notes?

@pes10k
Copy link
Contributor Author

pes10k commented Nov 2, 2020

I dont think this needs a manual QA step, but for release notes, the below should be good i think:

Change the HTTP error code Brave uses when blocking a network request to better match what other browsers and tools expect, to increase compatibility with crowdsourced filter lists

@bsclifton bsclifton changed the title Do what uBO does when a resource is blocked (i.e. onerror path) Change the HTTP error code Brave uses when blocking a network request to better match what other browsers and tools expect, to increase compatibility with crowdsourced filter lists Nov 2, 2020
@bsclifton bsclifton added the QA/No label Nov 2, 2020
@bsclifton
Copy link
Member

@pes10k awesome! updated 😄👌

@bsclifton
Copy link
Member

Re-opening after reverting with brave/brave-core#7028

@bsclifton
Copy link
Member

Re-opening; this got closed on accident from a rebase (doh)

PR in progress with brave/brave-core#7030

@kjozwiak
Copy link
Member

kjozwiak commented Nov 10, 2020

Verification PASSED on Samsung S10+ on Android 10 using 1.16.75 CR: 86.0.4240.193

Ensured that websites listed under brave/brave-core#7030 (comment) didn't cause Brave to crash on both mobile & desktop versions:


Verification passed on Asus Zenfone (x86) with Android 6.0 running 1.16.75 Bravex86.apk


Verification passed on Brave v1.16.75 on Samsung Galaxy Tab S5e (Android 9.0)


Verification on macOS with

Brave | 1.16.75 Chromium: 86.0.4240.193 (Official Build) (x86_64)
-- | --
Revision | 4b1dc838fc8d2b55f2147e7a642b20baf443da5c-refs/branch-heads/4240@{#1411}
OS | macOS Version 10.14.6 (Build 18G6032)

Verification passed on

Brave 1.16.75 Chromium: 86.0.4240.193 (Official Build) (64-bit)
Revision 4b1dc838fc8d2b55f2147e7a642b20baf443da5c-refs/branch-heads/4240@{#1411}
OS Windows 7 Service Pack 1 (Build 7601.24544)

Verified passed with

Brave	1.16.75 Chromium: 86.0.4240.193 (Official Build) (64-bit)
Revision	4b1dc838fc8d2b55f2147e7a642b20baf443da5c-refs/branch-heads/4240@{#1411}
OS	Linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment