Skip to content

Add Safari 26 content blocker triggers#113

Closed
hyeonjongyang wants to merge 2 commits intoAdguardTeam:masterfrom
hyeonjongyang:safari26-content-blocker-triggers
Closed

Add Safari 26 content blocker triggers#113
hyeonjongyang wants to merge 2 commits intoAdguardTeam:masterfrom
hyeonjongyang:safari26-content-blocker-triggers

Conversation

@hyeonjongyang
Copy link
Contributor

Add Safari 26 trigger support to the content blocker compiler, including request method matching and the new frame URL domain scoping fields.

Changes

  • Add new BlockerEntry.Trigger fields and encoding support:
    • request-method
    • if-frame-url
    • unless-frame-url
  • Add $method= parsing/validation for network rules (Safari 26+), normalizing method names and supporting |-separated lists.
  • Emit one content blocker entry per $method value (Safari trigger supports a single request-method per entry).
  • For Safari 26+, translate $domain scoping into if-frame-url / unless-frame-url patterns (including example.* TLD wildcard support).
  • Extend unit tests for Safari 26 version gating, request method handling, frame URL scoping, and JSON encoding.

Reference

Notes

  • Behavior for Safari < 26 remains unchanged; $method is rejected and domain scoping continues to use if-domain / unless-domain.

Copy link

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 To request another review, post a new comment with "/windsurf-review".

@hyeonjongyang
Copy link
Contributor Author

/windsurf-review

Copy link

@windsurf-bot windsurf-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 To request another review, post a new comment with "/windsurf-review".

public var isCheckThirdParty = false
public var isThirdParty = false
public var isMatchCase = false
public var requestMethods: [String] = []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using Set<String> instead of [String] for the requestMethods property. This would provide more efficient membership checking and eliminate the need for the manual duplicate check on line 219.

Comment on lines +514 to +522
for domain in domains {
if domain.utf8.last == Chars.WILDCARD {
let prefix = String(domain.dropLast(2))
let escaped = NSRegularExpression.escapedPattern(for: prefix)
result.append(#"^[^:]+://+([^:/]+\.)?\#(escaped)\.[^/:]+[/:]?"#)
} else {
result.append(try SimpleRegex.createRegexText(pattern: "||\(domain)^"))
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's inconsistent indentation in this method. The for loop has an extra level of indentation that doesn't match the surrounding code style.

Comment on lines +180 to +190
private static let supportedRequestMethods: Set<String> = [
"get",
"head",
"options",
"trace",
"put",
"delete",
"post",
"patch",
"connect",
]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving the supportedRequestMethods set inside the setRequestMethods method since it's only used there, or at least mark it as private static to better encapsulate it.

@ameshkov
Copy link
Member

Hi, thank you for the contribution!

Could you please split the PR in two parts - one for $method and one for if-frame-url/unless-frame-url?

While $method it should be rather straightforward, but theif-frame-url change is more complicated and requires some additional checks.

In theory, if-domain/unless-domain should be much easier to compile and parse and switching from it to if-frame-url/unless-frame-url everywhere will negatively affect the overall compilation time. This can be verified by using XCode Instruments similarly to how I've done it here. If the difference is considerable and if-frame-url does indeed negatively affects performance, I suggest switching to a hybrid approach and only using it for a subset of rules (TLD, regexes): #100

@ameshkov
Copy link
Member

Also, for every pull request please add a changelog entry.

Relevant issues that are addressed:

@hyeonjongyang
Copy link
Contributor Author

Thanks for the review!

I’ve split the changes into two separate PRs as requested: #114 and #115.

Each PR includes a changelog entry. I’m going to close this PR in favor of those two.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants