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

fix: apply transparency settings to WebContentsPreferences #31685

Merged
merged 2 commits into from Nov 3, 2021

Conversation

VerteDinde
Copy link
Member

@VerteDinde VerteDinde commented Nov 2, 2021

Description of Change

In #30193, we refactored WebContentsPreferences to store its values directly and centralize logic for default values. As part of that refactor, the transparency setting was removed, and any transparent values were instead stored in the background_color_ property (e.g., background_color_ = SK_ColorTRANSPARENT.

We handle for cases like this explicitly for browser windows here:

// Copy the backgroundColor to webContents.
v8::Local<v8::Value> value;
bool transparent = false;
if (options.Get(options::kBackgroundColor, &value)) {
web_preferences.SetHidden(options::kBackgroundColor, value);
} else if (options.Get(options::kTransparent, &transparent) && transparent) {
// If the BrowserWindow is transparent, also propagate transparency to the
// WebContents unless a separate backgroundColor has been set.
web_preferences.SetHidden(options::kBackgroundColor,
ToRGBAHex(SK_ColorTRANSPARENT));
}

However, this transparency handling only applies to browser windows, and does not take into account events where the WebContents or WebContentsPreferences are created independently from the browser window, such as "-will-add-new-contents" events.

This PR adds a transparency check to web_contents_preferences, where any passed-in transparency values are checked and set the background_color_ to SK_ColorTRANSPARENT.

It also slightly amends a check in WebContents::CreateFromWebPreferences to only set a color to the RenderWidgetHostView if a color value exists. Because we're creating a window from web preferences, which don't recognize transparency, this can override transparent web contents with a white background on the window itself.

This PR has been tested on a series of gists to make sure no regressions were caused:
Background Color (gist)
Transparent Main Window (gist)
Transparent Child Window (gist)

Checklist

Release Notes

Notes: Fixes an issue where transparency was not always set correctly on webContents.

@VerteDinde VerteDinde added semver/patch backwards-compatible bug fixes target/16-x-y labels Nov 2, 2021
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Nov 2, 2021
@electron electron deleted a comment from Jukraphunt Nov 3, 2021
@miniak
Copy link
Contributor

miniak commented Nov 3, 2021

@VerteDinde this PR does not fix #31686.

@VerteDinde
Copy link
Member Author

VerteDinde commented Nov 3, 2021

Milan and I took a look at this PR - I'm going to merge this to fix an open issue with child windows and "-will-add-new-contents", I think we'll have to take a slightly different code path for the webview issue. Changing the rwhv call to rwhv->SetBackgroundColor(SK_ColorTRANSPARENT) looks like it's causing a different issue

@VerteDinde VerteDinde merged commit 86f6285 into main Nov 3, 2021
@VerteDinde VerteDinde deleted the fix-transparency-on-web-contents branch November 3, 2021 18:16
@release-clerk
Copy link

release-clerk bot commented Nov 3, 2021

Release Notes Persisted

Fixes an issue where transparency was not always set correctly on webContents.

@trop
Copy link
Contributor

trop bot commented Nov 3, 2021

I have automatically backported this PR to "16-x-y", please check out #31700

t57ser pushed a commit to t57ser/electron that referenced this pull request Jan 25, 2022
…31685)

* fix: add transparency to web_contents_preferences

* fix: correctly apply transparency settings to new webContents from webPreferences
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-pr 🌱 PR opened in the last 24 hours semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants