Skip to content

In Standard Shields, significant text check should happen before partiness check #40967

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

Closed
StephenHeaps opened this issue Sep 10, 2024 · 2 comments · Fixed by brave/brave-core#25499

Comments

@StephenHeaps
Copy link

StephenHeaps commented Sep 10, 2024

Description

On iOS, in Standard mode, cosmetically filtered content is hidden as soon as possible by hiding their element subtree, then after the content is hidden, we do another pass to check the subtree of the all these hidden elements to see if there is a 1st party or if they contain significant text](https://github.com/brave/brave-core/blob/master/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Sandboxed/SelectorsPollerScript.js#L446-L470). Hidden elements with 1st party URLs are unhidden, and so is content with significant text. iOS has a logic bug that differs from desktop here, where if a hidden element contains a 3rd party resource, we fail to check for significant text.

Desktop logic for this 'should unhide hidden element' check checks if the hidden subtree is first party or shows significant text is here: https://github.com/brave/brave-core/blob/ed5f49a24e74176420a40263075ad3580a10f7a8/components/cosmetic_filters/resources/data/content_cosmetic.ts#L559-L569

On iOS the logic for this same 'should unhide hidden element' check is here:
https://github.com/brave/brave-core/blob/ed5f49a24e74176420a40263075ad3580a10f7a8/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Sandboxed/SelectorsPollerScript.js#L560-L581

The bug on iOS occurs when the hidden element contains a 3rd party resource AND significant text. In this case, queryResults.foundThirdPartyResource is true so we enter that if block first and never check for significant text:
https://github.com/brave/brave-core/blob/ed5f49a24e74176420a40263075ad3580a10f7a8/ios/brave-ios/Sources/Brave/Frontend/UserContent/UserScripts/Scripts_Dynamic/Scripts/Sandboxed/SelectorsPollerScript.js#L567-L574

This is where iOS differs in the 'should unhide hidden element' check. See https://github.com/brave/internal/issues/1188 for STR.

@kjozwiak
Copy link
Member

The above requires 1.69.170 or higher for 1.69.x verification 👍

@hffvld
Copy link
Contributor

hffvld commented Sep 11, 2024

Verified on iPhone 14 and iPad Air using version(s):

Device/OS: 
- iPhone 14 / iOS 17.6.1
- iPad Air / iPadOS 16.7.2
Brave build: 1.69 (170)
BraveCore: 1.69.170 (128.0.6613.138)

Verification notes can be found via https://github.com/brave/internal/issues/1188#issuecomment-2344831684 and https://github.com/brave/internal/issues/1188#issuecomment-2344977815.

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

Successfully merging a pull request may close this issue.

4 participants