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: defaultFontFamily in webPreferences #37863

Merged
merged 2 commits into from Apr 13, 2023

Conversation

dsanders11
Copy link
Member

@dsanders11 dsanders11 commented Apr 6, 2023

Description of Change

This fix brought to you by clang-tidy, which detected the problem:

../../electron/shell/browser/web_contents_preferences.cc:430:71: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [bugprone-string-integer-assignment]
    prefs->standard_font_family_map[blink::web_pref::kCommonScript] = font;
                                                                      ^
../../electron/shell/browser/web_contents_preferences.cc:433:68: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [bugprone-string-integer-assignment]
    prefs->serif_font_family_map[blink::web_pref::kCommonScript] = font;
                                                                   ^
../../electron/shell/browser/web_contents_preferences.cc:436:73: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [bugprone-string-integer-assignment]
    prefs->sans_serif_font_family_map[blink::web_pref::kCommonScript] = font;
                                                                        ^
../../electron/shell/browser/web_contents_preferences.cc:439:68: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [bugprone-string-integer-assignment]
    prefs->fixed_font_family_map[blink::web_pref::kCommonScript] = font;
                                                                   ^
../../electron/shell/browser/web_contents_preferences.cc:442:70: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [bugprone-string-integer-assignment]
    prefs->cursive_font_family_map[blink::web_pref::kCommonScript] = font;
                                                                     ^
../../electron/shell/browser/web_contents_preferences.cc:445:70: warning: an integer is interpreted as a character code when assigning it to a string; if this is intended, cast the integer to the appropriate character type; if you want a string representation, use the appropriate conversion facility [bugprone-string-integer-assignment]
    prefs->fantasy_font_family_map[blink::web_pref::kCommonScript] = font;

Got broken by the refactors in #30193 (which shipped in v16) and there was no test coverage.

As far as I can tell, though, no one has reported the issue in that time, so that may speak to how used this option is.

Checklist

Release Notes

Notes: Fixed an issue which made defaultFontFamily in webPreferences have no effect

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 6, 2023
@dsanders11 dsanders11 added semver/patch backwards-compatible bug fixes target/22-x-y PR should also be added to the "22-x-y" branch. target/23-x-y PR should also be added to the "23-x-y" branch. target/24-x-y PR should also be added to the "24-x-y" branch. labels Apr 6, 2023
@dsanders11 dsanders11 marked this pull request as ready for review April 6, 2023 11:48
@jkleinsc jkleinsc added the target/25-x-y PR should also be added to the "25-x-y" branch. label Apr 6, 2023
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 7, 2023
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

Since this option is rarely used it might worth adding a if (!default_font_family_.empty()) test.

@codebytere codebytere merged commit e84bcb3 into electron:main Apr 13, 2023
10 checks passed
@release-clerk
Copy link

release-clerk bot commented Apr 13, 2023

Release Notes Persisted

Fixed an issue which made defaultFontFamily in webPreferences have no effect

@trop
Copy link
Contributor

trop bot commented Apr 13, 2023

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

@trop trop bot removed the target/25-x-y PR should also be added to the "25-x-y" branch. label Apr 13, 2023
@trop
Copy link
Contributor

trop bot commented Apr 13, 2023

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

@trop
Copy link
Contributor

trop bot commented Apr 13, 2023

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

@trop
Copy link
Contributor

trop bot commented Apr 13, 2023

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

@trop trop bot added in-flight/23-x-y and removed target/24-x-y PR should also be added to the "24-x-y" branch. target/23-x-y PR should also be added to the "23-x-y" branch. labels Apr 13, 2023
@trop trop bot removed the target/22-x-y PR should also be added to the "22-x-y" branch. label Apr 13, 2023
@codebytere codebytere mentioned this pull request Apr 13, 2023
3 tasks
@dsanders11 dsanders11 deleted the fix-default-font-family branch April 13, 2023 13:54
@trop trop bot added merged/22-x-y PR was merged to the "22-x-y" branch. merged/23-x-y PR was merged to the "23-x-y" branch. merged/24-x-y PR was merged to the "24-x-y" branch and removed in-flight/22-x-y labels Apr 14, 2023
@trop trop bot added merged/25-x-y PR was merged to the "25-x-y" branch. and removed in-flight/25-x-y labels Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged/22-x-y PR was merged to the "22-x-y" branch. merged/23-x-y PR was merged to the "23-x-y" branch. merged/24-x-y PR was merged to the "24-x-y" branch merged/25-x-y PR was merged to the "25-x-y" branch. semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants