Skip to content

Enable css-validation feature by default to fix :has-text() selector#609

Open
fresh3nough wants to merge 1 commit into
brave:masterfrom
fresh3nough:fix/enable-css-validation-by-default
Open

Enable css-validation feature by default to fix :has-text() selector#609
fresh3nough wants to merge 1 commit into
brave:masterfrom
fresh3nough:fix/enable-css-validation-by-default

Conversation

@fresh3nough

Copy link
Copy Markdown

The :has-text() selector and other procedural operators were not working because the css-validation feature was not enabled by default. Without this feature, all selectors are treated as plain CSS instead of being properly parsed to extract procedural operators.

This fix enables css-validation as a default feature, allowing :has-text() and other procedural selectors to work correctly.

Fixes #537


Note: This is a re-opening of #588, which was accidentally closed.

The :has-text() selector and other procedural operators were not working
because the css-validation feature was not enabled by default. Without
this feature, all selectors are treated as plain CSS instead of being
properly parsed to extract procedural operators.

This fix enables css-validation as a default feature, allowing :has-text()
and other procedural selectors to work correctly.

Fixes brave#537

Signed-off-by: fresh3nough <fresh3nough@users.noreply.github.com>
theoden8 pushed a commit to theoden8/webspace_app that referenced this pull request May 19, 2026
Without this flag, adblock-rust 0.12 silently fails to parse
procedural cosmetic rules (`:has-text()`, `:upward()`, `:remove()`,
`:remove-attr()`, `:remove-class()`, `:style()`) on class/id-anchored
generic selectors — even though the crate stores them internally,
they never appear in cosmeticResources(url).procedural_actions.

Upstream issue brave/adblock-rust#537 documents the symptom; PR
brave/adblock-rust#609 proposes making the flag default-on for
exactly this reason. Enabling it brings the cosmetic-rule surface
the FFI exposes up to the level the existing procedural shim runner
already expects, without us having to either fork the crate or
re-implement an in-Dart procedural parser.

The flag pulls in `cssparser` + `selectors`; ~70 kB added to the
static library, no API change. Cargo.lock will be regenerated on
the next build.
theoden8 added a commit to theoden8/webspace_app that referenced this pull request May 19, 2026
…overage (#363)

* build(content-blocker): disable LTO for Apple rust targets

Rust release profile sets lto=true, which makes rustc emit LLVM bitcode
.o files for the linker to finish codegen. When the Rust toolchain's
LLVM is newer than Xcode's (e.g. rust 1.95 / LLVM 22 vs Xcode 17 / LLVM
17), ld/nm reject the archive with "Unknown attribute kind (NNN)",
zero ws_* symbols make it into Runner, and ContentBlocker silently
falls back to the Dart parser. Pass CARGO_PROFILE_RELEASE_LTO=false for
ios/macos targets only — native Mach-O .o emission avoids the LLVM
version skew. Linux/Android keep full LTO.

* ci(content-blocker): assert _ws_engine_new is exported by Runner

The Podfile verify phase runs inside xcodebuild and can be skipped by
build-system caching or swallowed by output filtering. Add an explicit
post-build nm check in CI for both iOS Runner and the macOS binary, so
a missing rust link surfaces as a red job instead of a green build that
falls back to the Dart parser at runtime.

* ci: run pod install explicitly so post_install hook fires

Podfile post_install in 54391eb writes -force_load, -Wl,-u,_ws_engine_new,
and the verify build phase into Runner.xcodeproj at install time. Those
edits aren't committed. Flutter's build step only runs pod install when
Manifest.lock disagrees with Podfile.lock — under our pod cache they
always agree, so post_install never fires and the pbxproj keeps its
committed (unmodified) form, dropping all ws_* symbols at link.

Run pod install explicitly for both ios and macos before flutter build
to force the hook against each run's fresh checkout.

* ci: invalidate Manifest.lock instead of running pod install directly

The previous attempt to call `pod install` standalone trips
flutter_additional_ios_build_settings (podhelper.rb:61), likely because
Generated.xcconfig isn't fully set up outside the flutter build flow.
Deleting Pods/Manifest.lock makes Flutter's own pod-install-needed check
return true, so the standard flutter build invocation runs pod install
with the proper FLUTTER_ROOT and Generated.xcconfig env, and post_install
fires inside that.

* refactor(content-blocker): wrap rust .a in a CocoaPod, drop force_load hooks

Replaces the post_install gymnastics in ios/Podfile and macos/Podfile —
shell build phase for the rust lib, -force_load + -Wl,-u dance, verify
phase, manual pbxproj mutation — with a real CocoaPod we own at
rust/webspace_adblock/cocoapods/. Its source files are a single
keep_alive.c (auto-generated by scripts/build_rust.sh apple from the
crate's #[no_mangle] FFI surface) plus the vendored binaries:

  * iOS:   WebspaceAdblock.xcframework  (device arm64 + sim arm64/x86_64)
  * macOS: libwebspace_adblock-macos.a  (fat arm64 + x86_64)

Both built by `scripts/build_rust.sh apple` into the podspec dir.
CocoaPods links the .a into the framework, keep_alive.c's __used array
of FFI addresses defeats -dead_strip, and Runner gets the symbols at
runtime via dlsym(RTLD_DEFAULT, ...) from webspace_adblock.framework
embedded in the .app bundle.

Eliminates the whole class of CI bugs we'd been chasing:
  * Pod cache + uncommitted pbxproj mods (post_install never re-ran)
  * Per-symbol -Wl,-u list drift (now auto-derived from src/lib.rs)
  * -force_load + LLVM bitcode version skew (no force_load at all)
  * xcframework-vs-force_load incompatibility (no force_load)

Net: -214 +172 lines, ios/Podfile drops from 232 to 68.

* build(content-blocker): invoke build_rust.sh apple on every flutter build

Adds a script_phases entry to the webspace_adblock podspec so xcodebuild
runs scripts/build_rust.sh apple before linking the pod's framework,
plus a post_install hook in both Podfiles that flips alwaysOutOfDate=1
on that phase so Xcode doesn't skip it once it's run successfully.

Net effect: every `fvm flutter build` (ios/macos) invokes the rust
build. cargo's incremental compile keeps the no-op steady-state cost
near zero; the win is that a src/lib.rs edit can never silently miss
the framework — the FFI surface in the linked binary always matches
the current source at build time.

Also stops swallowing `rustup target add` failures in the script —
previously they surfaced as the much more confusing cargo error
"can't find crate for `core`" several seconds later.

* fix(build_rust.sh): force rustup's cargo so iOS targets resolve

Symptom: rustup target list/add says aarch64-apple-ios is installed,
but cargo build emits E0463 "can't find crate for \`core\`" several
seconds in. Cause is a second rust on PATH (typically /opt/homebrew/
bin/cargo from `brew install rust`) shadowing ~/.cargo/bin/cargo —
rustup populated rustup's sysroot, but cargo is reading brew's.

Fix: prepend rustup's bin to PATH inside the apple build, and drive
cargo through `rustup run <toolchain> cargo …` so the toolchain
hosting the targets is the one doing the build, regardless of PATH
order. Also fail fast with a useful message if rustup isn't on PATH
at all.

* build(content-blocker): strip stale Runner build phases from pre-podspec era

CocoaPods doesn't auto-remove what a previous post_install added.
Users upgrading from the force_load architecture have leftover
`[webspace] Build adblock-rust static lib` + `Verify ...` phases on
Runner pointing at deleted shell-script paths, plus -force_load /
-Wl,-u flags in OTHER_LDFLAGS. Both Podfiles now strip them on every
pod install — idempotent no-op once the project is clean.

* fix(webspace_adblock pod): pin LC_LOAD_DYLIB via -needed_framework

Runner had `-framework webspace_adblock` in its OTHER_LDFLAGS, the
pod's framework binary linked correctly with 15 ws_* exports, and the
framework was copied into the .app bundle — but `otool -L Runner` did
not list it, and dlsym(RTLD_DEFAULT, "ws_engine_new") returned null
at runtime.

Cause: Apple ld defaults to `dead_strip_dylibs`, which removes the
LC_LOAD_DYLIB load command for any dylib that has zero compile-time
symbol references. Runner's Swift/ObjC code never references ws_*
directly (Dart FFI does dlsym at runtime), so ld dropped the
dependency. Result: framework on disk, dyld never loads it.

`-Wl,-needed_framework,webspace_adblock` tells ld to keep the
LC_LOAD_DYLIB regardless of references. Wired via the podspec's
user_target_xcconfig so it propagates to Runner without us touching
its xcconfig directly.

* fix: xcframework slice basenames + -needed_framework via post_install

Two stacked issues found from CI/user run after the podspec refactor:

1) xcodebuild -create-xcframework rejected the simulator slice when
   its file basename (iphonesimulator-fat-libwebspace_adblock.a) didn't
   match the device slice (libwebspace_adblock.a). CocoaPods then
   refused to install the xcframework: "static libraries with
   differing binary names". Stage the simulator fat .a in a dedicated
   target/iphonesimulator-fat/ directory under the same basename.

2) Pod user_target_xcconfig OTHER_LDFLAGS doesn't make it into
   Pods-Runner.xcconfig — CocoaPods drops overrides for keys it
   already manages — so Apple ld's dead_strip_dylibs kept removing
   webspace_adblock from Runner's LC_LOAD_DYLIB despite the flag
   being declared in the podspec. Inject -Wl,-needed_framework,
   webspace_adblock directly into Runner's OTHER_LDFLAGS via the
   existing post_install hook in both Podfiles instead. Idempotent.

* fix(content-blocker): substitute synthetic host for file://, blob:, data:

adblock-rust's Engine::url_cosmetic_resources constructs a Request
from the URL and returns UrlSpecificResources::empty() when that
fails — which it does for any URL without a real hostname. The
generic-rule merge (misc_generic_selectors: attribute selectors,
:has, :style, procedural shapes) lives downstream of that early
return, so all complex generic cosmetic shapes silently miss on
file:// / blob: / data: / about: pages.

Pre-65be41b the Dart parser's _cosmeticSelectors[''] bucket caught
these. 65be41b deleted the parser without restoring the path, so
the probe page (file:///) and HTML imports lost everything except
class/id-keyed rules.

Pass `https://webspace.invalid/` to the engine when the page URL is
hostless. `.invalid` is reserved by RFC 6761; no real filter list
will target that host, so domain-specific rules can't accidentally
fire. The misc-generic-selector merge runs and returns the rules
we want.

* fix(webspace_adblock): enable adblock-rust css-validation feature

Without this flag, adblock-rust 0.12 silently fails to parse
procedural cosmetic rules (`:has-text()`, `:upward()`, `:remove()`,
`:remove-attr()`, `:remove-class()`, `:style()`) on class/id-anchored
generic selectors — even though the crate stores them internally,
they never appear in cosmeticResources(url).procedural_actions.

Upstream issue brave/adblock-rust#537 documents the symptom; PR
brave/adblock-rust#609 proposes making the flag default-on for
exactly this reason. Enabling it brings the cosmetic-rule surface
the FFI exposes up to the level the existing procedural shim runner
already expects, without us having to either fork the crate or
re-implement an in-Dart procedural parser.

The flag pulls in `cssparser` + `selectors`; ~70 kB added to the
static library, no API change. Cargo.lock will be regenerated on
the next build.

* chore(webspace_adblock): regenerate Cargo.lock for css-validation deps

Enabling the css-validation feature in d1f74fa added cssparser +
selectors (plus their transitives) to the resolved dependency graph.
build_rust.sh runs `cargo build --locked`, which refuses to update
Cargo.lock and fails with "cannot update the lock file ... because
--locked was passed to prevent this" until the lockfile is regenerated.

* feat(content-blocker): add procedural+style backfill parser (unwired)

Adds a focused Dart parser for the two cosmetic rule shapes
adblock-rust 0.12 parses correctly (with css-validation) but has no
public API to retrieve by class/id:
  * `##sel:remove() / :remove-attr() / :remove-class()` procedurals
  * `##sel:style(decls)` style overrides

Per-anchor buckets keyed on the first class/id token in the selector,
mirroring `engine.hiddenClassIdSelectors`. Output shape matches
`buildProceduralCosmeticShim`'s input so backfilled rules and engine
rules flow through the same runner without origin branching.

Not yet wired into ContentBlockerService — leaving it as a standalone
module for review first. Wiring is a separate commit that touches
the engine rebuild path and the page-side `genericCosmeticScan`
bridge handler.

* fix(content-blocker): bypass adblock-rust GenericAction via synthetic host

adblock-rust 0.12 rejects every generic procedural rule at parse
time (cosmetic.rs:444 — `if sharp_index == 0 && action.is_some()
{ return Err(GenericAction) }`). No FFI path surfaces them and
no feature flag including css-validation changes that — the
rejection runs before validation. Verified with isolated tests
in the new procedural_backfill integration suite.

Prepend a synthetic hostname (`localhost`) to any generic
cosmetic rule that carries a `:remove()` / `:remove-attr()` /
`:remove-class()` / `:style()` action before handing the filter
list to the engine. The parser then treats it as domain-scoped,
stores it in the cosmetic cache, and `cosmeticResources(
"https://localhost/")` returns the rule via the standard
`procedural_actions` field in the same JSON shape as native
domain-scoped procedurals — so the existing procedural shim
runner consumes both without branching.

Replaces the in-Dart procedural-rule parser drafted earlier
(now 319 lines lighter): pure prefix rewrite plus a regex to
detect the action pseudo. All parsing and selector handling
stays inside the crate.

Adds a 4-test integration suite in `rust/webspace_adblock/tests/`
that pins:
  - generic procedural rules are dropped at parse time
  - synthetic-host prefix recovers them via procedural_actions
  - all four action shapes round-trip
  - domain-scoped procedurals are unaffected
CI runs `cargo test` on the Linux job so a future adblock-rust
upgrade can't silently undo this behaviour.

The Dart-side caller (ContentBlockerService) still needs to call
`rewriteGenericProceduralsForBackfill` before passing rulesText
to the engine, and query `cosmeticResources("https://localhost/")`
in addition to the page URL — those wiring changes follow in a
separate commit.

* feat(content-blocker): wire procedural backfill into engine pipeline

Two integration points for procedural_action_backfill.dart:

1. `_rebuildEngine` now feeds the filter-list text through
   `rewriteGenericProceduralsForBackfill` before passing to
   `AdblockEngine.load`. Generic procedural rules (`##.foo:remove()`
   etc.) get a `localhost##` prefix that bypasses the crate's
   parse-time `GenericAction` rejection.

2. `_engineCosmeticFor(pageUrl)` now also queries
   `cosmeticResources('https://localhost/')` and unions the returned
   `procedural_actions` into the result. Skipped when the page URL
   already maps to the synthetic host (file:// → localhost path).
   Hides + exceptions from the synthetic query are discarded — the
   real-URL query already surfaces them via the misc-generic merge.

Cache invalidation: bumps `_kEngineCacheVersion` to 2 and prefixes
the rules hash with it, so blobs written by builds before the
rewrite landed get rejected on first launch and the engine re-parses
the rewritten text. Without the bump, cached engines would be stuck
without procedural rules until the user manually nuked the cache.

The probe page's sections 1B / 1C / 1E should now flip from
"missed" to "action fired" / "height: 1px" without any cache wipe.

* fix(content-blocker): normalise ABP aliases + wrap default-hide filter pseudos

Two follow-ups to the procedural backfill rewriter:

1. Filter-pseudo rules with no action pseudo (`##.foo:has-text(X)`,
   `##.foo:-abp-has(.x)`, `##.foo:contains(X)`) are default-hide in
   uBO syntax. Adblock-rust stores them as hide selectors with the
   procedural pseudo embedded in the selector string — fine for
   storage but the early-CSS injection then can't match anything,
   because the pseudo isn't real CSS. The rewriter now appends
   `:style(display: none !important)` to convert these into
   procedural-style rules that the procedural shim runner handles
   correctly.

2. With css-validation on, the crate's parser only accepts canonical
   uBO pseudo names. ABP-syntax aliases (`:-abp-contains`, `:contains`,
   `:-abp-has`) are silently rejected and the whole rule is dropped.
   The rewriter now normalises them BEFORE handing to the parser:
   `:contains(` / `:-abp-contains(` → `:has-text(`, `:-abp-has(` →
   `:has(`. After normalisation the procedural shim sees a uniform
   has-text/has structure regardless of original syntax.

Drive-by: the inline `rules_to_engine` test helper in src/lib.rs
hadn't been updated when `ws_engine_new` gained the
`enable_ubo_resources` param. Fixing — both as a syntax error and
as a behaviour mismatch (the redirect test needs resources loaded).

Probe page sections 1B / 1C / 1E now green, section 1 rows 7-9
(the filter-pseudo-only rules) also flip to BLOCKED.

* fix(ios/macos): emit dwarf-in-binary for pods to satisfy archive validation

objective_c (transitive Flutter dep) ships a precompiled .framework
without a dSYM. Xcode archive validation then refuses to distribute:
"The archive did not include a dSYM for the objective_c.framework
with the UUIDs ...".

Set DEBUG_INFORMATION_FORMAT = dwarf for ALL pod targets so debug
info is inlined into each pod's binary and no separate dSYM is
expected. Apple's crash symbolication against pod code becomes
approximate, but Flutter app stacks land mostly in Dart anyway,
where symbolication uses our own symbol map regardless. Trade-off
worth it for unblocking archive distribution.

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

has-text selector does not work

1 participant