Conversation
Co-authored-by: Mr. M <mr.m@tuta.com> Co-authored-by: mr. m <91018726+mr-cheffy@users.noreply.github.com>
I've added a fix for the zapping which will now only set the display to none of a zapped element after the dissolve animation is fully complete. I also plan on looking forward to fixing the color invalidation issue, which is not done yet.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “Zen Boosts” feature set, spanning UI entry points (site identity panel + editor window), new JSWindowActors for applying boosts in content, and a C++ backend hook to tint/invert rendered colors based on per-tab boost state stored on BrowsingContext.
Changes:
- Add Boosts UI in the site data panel (create/edit/toggle, boosted indicator animation).
- Add content/parent JS actors + supporting overlays (selector/picker + zap UI) and styling assets.
- Add a native color-filtering backend (
nsZenBoostsBackend) and upstream patch hooks to apply boost filtering during style/painting.
Reviewed changes
Copilot reviewed 48 out of 68 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/zen/urlbar/ZenSiteDataPanel.sys.mjs | Adds boost UI integration, boost list, and boosted indicator updates. |
| src/zen/tests/moz.build | Registers new browser-chrome test manifest for boosts. |
| src/zen/tests/boosts/head.js | Test head importing SelectorComponent. |
| src/zen/tests/boosts/browser_boost_selector_nthchild.js | Adds selector path nth-child coverage tests. |
| src/zen/tests/boosts/browser_boost_selector_invalid.js | Adds invalid-node selector path tests. |
| src/zen/tests/boosts/browser_boost_selector_basic.js | Adds baseline selector path tests. |
| src/zen/tests/boosts/browser.toml | Defines the boosts test manifest + support files. |
| src/zen/spaces/ZenGradientGenerator.mjs | Emits observer notifications on gradient updates and adjusts workspace theme assignment timing. |
| src/zen/moz.build | Adds the new boosts directory to the build. |
| src/zen/mods/nsZenModsBackend.cpp | Refactors safe-mode detection into an if-init. |
| src/zen/images/boost-indicator.svg | Adds boosted indicator animation asset. |
| src/zen/drag-and-drop/nsZenDragAndDrop.h | Renames DnD contract ID macro. |
| src/zen/drag-and-drop/nsZenDragAndDrop.cpp | Updates service lookup to new DnD contract ID macro. |
| src/zen/common/sys/ZenActorsManager.sys.mjs | Registers ZenBoosts JSWindowActor when not in safe mode. |
| src/zen/common/styles/zen-single-components.css | Adds boost list item/editor button styles and shared permission/boost styling. |
| src/zen/common/styles/zen-popup.css | Extends popup padding styling to boost items. |
| src/zen/boosts/zen-zap.css | Adds zap overlay UI styling. |
| src/zen/boosts/zen-selector.css | Adds selector/picker overlay UI styling. |
| src/zen/boosts/zen-boosts.css | Adds boost editor window styling. |
| src/zen/boosts/zen-boost-editor.inc.xhtml | Adds boost editor window markup and bootstrapping script. |
| src/zen/boosts/zen-advanced-color-options.css | Adds advanced color options panel styling. |
| src/zen/boosts/nsZenBoostsBackend.h | Declares native boosts backend and filtering APIs. |
| src/zen/boosts/nsZenBoostsBackend.cpp | Implements native color filtering + invert logic based on BrowsingContext. |
| src/zen/boosts/nsZenBCOverrides.cpp | Adds BrowsingContext::DidSet hooks to trigger restyles when boost fields change. |
| src/zen/boosts/moz.build | Wires boosts JS modules, actors, and native sources into the build. |
| src/zen/boosts/jar.inc.mn | Packages boosts styles/editor window and boost indicator image. |
| src/zen/boosts/actors/ZenBoostsParent.sys.mjs | Parent actor: observes boost-related topics and serves boost data to content. |
| src/zen/boosts/actors/ZenBoostsChild.sys.mjs | Child actor: applies boost styles/data to BrowsingContext, manages zap/picker overlays. |
| src/zen/boosts/ZenZapOverlayChild.sys.mjs | Implements zap overlay UI and selector integration. |
| src/zen/boosts/ZenZapDissolve.sys.mjs | Implements WebGL dissolve effect for zap actions. |
| src/zen/boosts/ZenSelectorComponent.sys.mjs | Implements on-page element selector/picker and CSS path generation. |
| src/zen/boosts/ZenBoostStyles.sys.mjs | Generates per-boost CSS and caches style URIs. |
| src/layout/style/StyleColor-cpp.patch | Patches style color resolution to run through boosts filtering. |
| src/layout/painting/nsDisplayList-cpp.patch | Patches pres-shell entry to update boosts backend browsing-context tracking. |
| src/layout/generic/ViewportFrame-cpp.patch | Patches top-layer build to flag anonymous content for boosts backend. |
| src/layout/base/PresShell-cpp.patch | Patches default background color to be boosts-filtered. |
| src/gfx/layers/AnimationInfo-cpp.patch | Patches animated color extraction to be boosts-filtered. |
| src/dom/chrome-webidl/BrowsingContext-webidl.patch | Adds zenBoostsData and isZenBoostsInverted WebIDL attributes. |
| src/docshell/base/BrowsingContext-h.patch | Adds browsing context fields + DidSet hooks for boosts. |
| src/browser/themes/shared/zen-icons/nucleo/wand-sparkle.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/text-uppercase.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/text-title-case.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/text-size.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/text-lowercase.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/square-wand-sparkle.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/sliders.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/paintbrush.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/paintbrush-fill.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/lightbulb.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/hammer.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/eyedropper.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/close-filled-round.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/brackets-curly.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/boost.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/bolt.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/blocked-element.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/block.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/nucleo/arrow-rotate-anticlockwise.svg | Adds new boost-related icon asset. |
| src/browser/themes/shared/zen-icons/jar.inc.mn | Packages new icon assets into the theme jar. |
| src/browser/themes/shared/zen-icons/icons.css | Adds icons and styling hooks for boosts UI elements. |
| src/browser/base/content/zen-panels/site-data.inc | Adds boosts section and create-boost button to site-data panel. |
| src/browser/base/content/zen-locales.inc.xhtml | Registers zen-boosts.ftl localization bundle. |
| src/browser/base/content/zen-assets.jar.inc.mn | Includes boosts jar packaging in browser assets. |
| prefs/zen/boosts.yaml | Adds boosts-related preferences (enabled, dissolve, invert floor, anon-content disable). |
| locales/en-US/browser/browser/zen-boosts.ftl | Adds English strings for boosts UI/overlays. |
| crowdin.yml | Adds boosts FTL to Crowdin translation configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (boostData.autoTheme) { | ||
| // Workspace color is converted to the HSL color space | ||
| let primaryGradientColor = boost.workspaceGradient[0]?.c ?? [ | ||
| 0, 0, 0.6, | ||
| ]; | ||
| boost.workspaceGradient.forEach(color => { | ||
| if (color.isPrimary) { | ||
| primaryGradientColor = this.#rgbToHsl( | ||
| color.c[0], | ||
| color.c[1], | ||
| color.c[2] | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| // Workspace color is converted back to rgb | ||
| // using the same modifiers as the color above | ||
| primaryGradientColor = this.#hslToRgb( | ||
| primaryGradientColor[0] / 360, | ||
| primaryGradientColor[1] * (1 - boostData.saturation), | ||
| 0.1 + primaryGradientColor[2] * 0.9 * boostData.brightness | ||
| ); | ||
|
|
There was a problem hiding this comment.
Auto-theme path: primaryGradientColor is initialized from workspaceGradient[0]?.c (which can be an RGB string for custom colors) and is only converted to HSL when an isPrimary entry is found. If isPrimary is missing or c is not an RGB array, the later #hslToRgb(...) call will use incompatible data. Normalize by always selecting a color (primary or fallback) and converting it to HSL first (and support string colors).
| invalidateStyleForDomain(domain) { | ||
| if (this.#stylesCache.has(domain)) { | ||
| const { uri } = this.#stylesCache.get(domain); | ||
| lazy.styleSheetService.unregisterSheet(uri, AGENT_SHEET); | ||
| this.#stylesCache.delete(domain); | ||
| } |
There was a problem hiding this comment.
invalidateStyleForDomain() calls nsIStyleSheetService.unregisterSheet(...), but this module never registers the sheet with styleSheetService (it only returns a data: URI and the actor uses windowUtils.loadSheet/removeSheet). This unregister call is likely a no-op at best or can throw if the sheet wasn't registered. Consider removing the unregister call or registering/unregistering consistently.
| let counter = 0; | ||
| elements.forEach(async element => { | ||
| if (counter > this.#dissolvePoolSize) { | ||
| return; | ||
| } | ||
| counter++; | ||
|
|
There was a problem hiding this comment.
Off-by-one: the dissolve pool size limit check uses if (counter > this.#dissolvePoolSize) which allows #dissolvePoolSize + 1 elements to be processed. Use >= (and consider short-circuiting before increment) to cap the number of dissolve animations correctly.
| /* This Source Code Form is subject to the terms of the Mozilla Public | ||
| * License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ | ||
|
|
||
| #include "nsZenBoostsBackend.h" | ||
|
|
||
| #include "nsIXULRuntime.h" | ||
| #include "nsPresContext.h" | ||
|
|
||
| #include "mozilla/ClearOnShutdown.h" | ||
| #include "mozilla/StaticPtr.h" | ||
|
|
||
| #include "mozilla/ServoStyleConsts.h" | ||
| #include "mozilla/ServoStyleConstsInlines.h" | ||
| #include "mozilla/MediaFeatureChange.h" | ||
|
|
||
| #include "mozilla/dom/Document.h" | ||
| #include "mozilla/dom/DocumentInlines.h" | ||
| #include "mozilla/dom/BrowsingContext.h" | ||
|
|
||
| #include "mozilla/StaticPrefs_zen.h" | ||
|
|
||
| // Lower bound applied to inverted channels so that pure white doesn't invert | ||
| // all the way to pure black, which makes inverted pages feel too dark. | ||
| #define INVERT_CHANNEL_FLOOR() \ | ||
| (mozilla::StaticPrefs::zen_boosts_invert_channel_floor_AtStartup()) | ||
|
|
||
| #define SHOULD_APPLY_BOOSTS_TO_ANONYMOUS_CONTENT() \ | ||
| (!mozilla::StaticPrefs::zen_boosts_disable_on_anonymous_content_AtStartup()) | ||
|
|
||
| #if defined(__clang__) || defined(__GNUC__) | ||
| # define ZEN_HOT_FUNCTION __attribute__((hot)) | ||
| #else | ||
| # define ZEN_HOT_FUNCTION | ||
| #endif | ||
|
|
||
| // It's a bit of a hacky solution, but instead of using alpha as what it is | ||
| // (opacity), we use it to store contrast information for now. | ||
| // We do this primarily to avoid having to deal with WebIDL structs and | ||
| // serialization/deserialization between parent and content processes. | ||
| #define NS_GET_CONTRAST(_c) NS_GET_A(_c) | ||
|
|
||
| namespace zen { | ||
|
|
||
| nsZenAccentOklab nsZenBoostsBackend::mCachedAccent{0}; | ||
|
|
||
| namespace { | ||
|
|
||
| /** | ||
| * @brief Converts an sRGB color component to linear space. | ||
| * @param c The sRGB color component value (0.0 to 1.0). | ||
| * @return The linear color component value. | ||
| */ | ||
| static inline float srgbToLinear(float c) { | ||
| return c <= 0.04045f ? c * (1.0f / 12.92f) | ||
| : std::pow((c + 0.055f) * (1.0f / 1.055f), 2.4f); | ||
| } | ||
|
|
||
| /** | ||
| * @brief Converts a linear color component to sRGB space. | ||
| * @param c The linear color component value. | ||
| * @return The sRGB color component value (0.0 to 1.0). | ||
| */ | ||
| static inline float linearToSrgb(float c) { | ||
| c = std::max(0.0f, c); | ||
| return c <= 0.0031308f ? 12.92f * c | ||
| : 1.055f * std::pow(c, 1.0f / 2.4f) - 0.055f; | ||
| } | ||
|
|
||
| /* | ||
| * @brief Fast approximation of the cube root of a number. | ||
| * @param x The input value. | ||
| * @return The approximate cube root of the input value. | ||
| */ | ||
| static inline float fastCbrt(float x) { | ||
| if (x == 0.0f) return 0.0f; | ||
| float a = std::abs(x); | ||
| union { | ||
| float f; | ||
| uint32_t i; | ||
| } u = {a}; | ||
| u.i = u.i / 3 + 0x2a504a2e; | ||
| float y = u.f; | ||
| y = (2.0f * y + a / (y * y)) * (1.0f / 3.0f); | ||
| y = (2.0f * y + a / (y * y)) * (1.0f / 3.0f); | ||
| return x < 0.0f ? -y : y; | ||
| } |
There was a problem hiding this comment.
This file uses std::pow, std::cos, std::sin, std::abs, std::max/min, std::clamp, and uint32_t but does not include the corresponding standard headers. Add the needed includes (e.g. <algorithm>, <cmath>, <cstdint>) to avoid relying on transitive includes and potential build failures.
| /** | ||
| * @brief Called when a presshell is entered during rendering. | ||
| * @param aPresContext The presentation context that was entered. | ||
| */ | ||
| auto onPresShellEntered(mozilla::dom::Document* aDocument) -> void; | ||
|
|
There was a problem hiding this comment.
Header doc mismatch: onPresShellEntered takes a Document* aDocument, but the comment still refers to aPresContext. Update the doc comment so it matches the actual parameter and behavior.
| if (boost) { | ||
| const { boostData } = boost.boostEntry; | ||
| if (boost.styleSheet) { | ||
| this.#loadStyleSheet(boost.styleSheet); | ||
| } | ||
|
|
||
| browsingContext.isZenBoostsInverted = boostData.smartInvert; | ||
| if (boostData.enableColorBoost) { | ||
| if (boostData.autoTheme) { | ||
| // Workspace color is converted to the HSL color space | ||
| let primaryGradientColor = boost.workspaceGradient[0]?.c ?? [ | ||
| 0, 0, 0.6, | ||
| ]; | ||
| boost.workspaceGradient.forEach(color => { | ||
| if (color.isPrimary) { | ||
| primaryGradientColor = this.#rgbToHsl( | ||
| color.c[0], | ||
| color.c[1], | ||
| color.c[2] | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| // Workspace color is converted back to rgb | ||
| // using the same modifiers as the color above | ||
| primaryGradientColor = this.#hslToRgb( | ||
| primaryGradientColor[0] / 360, | ||
| primaryGradientColor[1] * (1 - boostData.saturation), | ||
| 0.1 + primaryGradientColor[2] * 0.9 * boostData.brightness | ||
| ); | ||
|
|
||
| const rgbColor = primaryGradientColor; | ||
| const nsColor = this.#rgbToNSColor( | ||
| rgbColor, | ||
| (1 - boostData.contrast) * 255 | ||
| ); | ||
| browsingContext.zenBoostsData = nsColor; | ||
| } else { | ||
| let colorWheelColor = this.#hslToRgb( | ||
| boostData.dotAngleDeg / 360, | ||
| /* already is [0, 1] */ | ||
| boostData.dotDistance * (1 - boostData.saturation), | ||
| /* lightness range from [0.1, 0.9] */ | ||
| 0.1 + boostData.dotDistance * 0.8 * boostData.brightness | ||
| ); | ||
|
|
||
| const rgbColor = colorWheelColor; | ||
| const nsColor = this.#rgbToNSColor( | ||
| rgbColor, | ||
| (1 - boostData.contrast) * 255 | ||
| ); | ||
| browsingContext.zenBoostsData = nsColor; | ||
| } | ||
| return; | ||
| } | ||
| } | ||
| browsingContext.zenBoostsData = 0; | ||
| } |
There was a problem hiding this comment.
When there is no boost (or color boost is disabled), the code clears browsingContext.zenBoostsData but does not reset browsingContext.isZenBoostsInverted. This can leave smart-invert enabled after navigating away or disabling a boost. Explicitly set isZenBoostsInverted = false in the no-boost path.
| dissolve(element, onComplete) { | ||
| if (!this.#initialized || this.#hasTriggered || !element) { | ||
| return; | ||
| } | ||
| this.#hasTriggered = true; | ||
|
|
||
| this.#onComplete = onComplete; | ||
|
|
||
| const rect = element.getBoundingClientRect(); | ||
| if (rect.width === 0 || rect.height === 0) { | ||
| console.warn("[ZapDissolve]: element has zero size. Skipping dissolve"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
ZapDissolve.dissolve() sets #hasTriggered = true before validating the target element's bounds. If the element has zero size, the function returns early and the instance remains permanently "triggered" and unusable. Move the #hasTriggered assignment until after validation or ensure it is reset on early returns.
| #initBrowserListeners() { | ||
| Services.obs.addObserver(this, "zen-boosts-update"); | ||
| this.window.gBrowser.addProgressListener({ | ||
| onLocationChange: aWebProgress => { | ||
| if (aWebProgress.isTopLevel) { | ||
| this.checkIfTabIsBoosted(); | ||
| } | ||
| }, | ||
| }); |
There was a problem hiding this comment.
#initBrowserListeners() adds a gBrowser.addProgressListener(...) using an anonymous object that is never removed on unload, which can leak listeners across window lifetime. Store the listener on this and remove it in the existing unload handler (and consider using the corresponding remove API for the same listener type).
| const enabled = boost.id === activeBoostId; | ||
| list.appendChild( | ||
| this.#createBoostPanelItem( | ||
| "boost-brush", | ||
| boostData.boostName, | ||
| enabled ? "Enabled" : "Disabled", | ||
| "zen-site-data-toggle-boost", | ||
| boost, |
There was a problem hiding this comment.
#setSiteBoost() uses hard-coded UI strings ("Enabled"/"Disabled") for the boost state label. These should be localized via Fluent (FTL) like the rest of the panel to avoid non-localizable UI.
| case "ZenBoost:ZapModeAny": { | ||
| const { boostData } = (await this.getWebsiteBoost()).boostEntry; | ||
| return !!boostData.zapSelectors.length; | ||
| } |
There was a problem hiding this comment.
ZenBoost:ZapModeAny assumes getWebsiteBoost() returns a non-null boost and will throw when no boost exists for the current domain. Handle the null case (and/or missing boostEntry) before dereferencing.
No description provided.