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: about panel crash #37373

Merged
merged 2 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shell/browser/browser.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ class Browser : public WindowListObserver {
base::Time last_dock_show_;
#endif

base::Value about_panel_options_;
base::Value::Dict about_panel_options_;

#if BUILDFLAG(IS_WIN)
void UpdateBadgeContents(HWND hwnd,
Expand Down
24 changes: 9 additions & 15 deletions shell/browser/browser_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -162,31 +162,25 @@ bool Browser::IsEmojiPanelSupported() {
void Browser::ShowAboutPanel() {
const auto& opts = about_panel_options_;

if (!opts.is_dict()) {
deepak1556 marked this conversation as resolved.
Show resolved Hide resolved
LOG(WARNING) << "Called showAboutPanel(), but didn't use "
"setAboutPanelSettings() first";
return;
}

GtkWidget* dialogWidget = gtk_about_dialog_new();
GtkAboutDialog* dialog = GTK_ABOUT_DIALOG(dialogWidget);

const std::string* str;
const base::Value* val;
const base::Value::List* list;

if ((str = opts.FindStringKey("applicationName"))) {
if ((str = opts.FindString("applicationName"))) {
gtk_about_dialog_set_program_name(dialog, str->c_str());
}
if ((str = opts.FindStringKey("applicationVersion"))) {
if ((str = opts.FindString("applicationVersion"))) {
gtk_about_dialog_set_version(dialog, str->c_str());
}
if ((str = opts.FindStringKey("copyright"))) {
if ((str = opts.FindString("copyright"))) {
gtk_about_dialog_set_copyright(dialog, str->c_str());
}
if ((str = opts.FindStringKey("website"))) {
if ((str = opts.FindString("website"))) {
gtk_about_dialog_set_website(dialog, str->c_str());
}
if ((str = opts.FindStringKey("iconPath"))) {
if ((str = opts.FindString("iconPath"))) {
GError* error = nullptr;
constexpr int width = 64; // width of about panel icon in pixels
constexpr int height = 64; // height of about panel icon in pixels
Expand All @@ -203,9 +197,9 @@ void Browser::ShowAboutPanel() {
}
}

if ((val = opts.FindListKey("authors"))) {
if ((list = opts.FindList("authors"))) {
std::vector<const char*> cstrs;
for (const auto& authorVal : val->GetList()) {
for (const auto& authorVal : *list) {
if (authorVal.is_string()) {
cstrs.push_back(authorVal.GetString().c_str());
}
Expand All @@ -223,7 +217,7 @@ void Browser::ShowAboutPanel() {
}

void Browser::SetAboutPanelOptions(base::Value::Dict options) {
about_panel_options_ = base::Value(std::move(options));
about_panel_options_ = std::move(options);
}

} // namespace electron
7 changes: 3 additions & 4 deletions shell/browser/browser_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,7 @@ bool CheckLoginItemStatus(bool* is_hidden) {
}

void Browser::ShowAboutPanel() {
NSDictionary* options =
DictionaryValueToNSDictionary(about_panel_options_.GetDict());
NSDictionary* options = DictionaryValueToNSDictionary(about_panel_options_);

// Credits must be a NSAttributedString instead of NSString
NSString* credits = (NSString*)options[@"Credits"];
Expand All @@ -537,13 +536,13 @@ bool CheckLoginItemStatus(bool* is_hidden) {
}

void Browser::SetAboutPanelOptions(base::Value::Dict options) {
about_panel_options_.GetDict().clear();
about_panel_options_.clear();

for (const auto pair : options) {
std::string key = pair.first;
if (!key.empty() && pair.second.is_string()) {
key[0] = base::ToUpperASCII(key[0]);
about_panel_options_.GetDict().Set(key, pair.second.Clone());
about_panel_options_.Set(key, pair.second.Clone());
}
}
}
Expand Down
17 changes: 8 additions & 9 deletions shell/browser/browser_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -734,30 +734,29 @@ void Browser::ShowEmojiPanel() {
}

void Browser::ShowAboutPanel() {
base::Value dict(base::Value::Type::DICTIONARY);
base::Value::Dict dict;
std::string aboutMessage = "";
gfx::ImageSkia image;

// grab defaults from Windows .EXE file
std::unique_ptr<FileVersionInfo> exe_info = FetchFileVersionInfo();
dict.SetStringKey("applicationName", exe_info->file_description());
dict.SetStringKey("applicationVersion", exe_info->product_version());
dict.Set("applicationName", exe_info->file_description());
dict.Set("applicationVersion", exe_info->product_version());

if (about_panel_options_.is_dict()) {
dict.MergeDictionary(&about_panel_options_);
}
// Merge user-provided options, overwriting any of the above
dict.Merge(about_panel_options_.Clone());

std::vector<std::string> stringOptions = {
"applicationName", "applicationVersion", "copyright", "credits"};

const std::string* str;
for (std::string opt : stringOptions) {
if ((str = dict.FindStringKey(opt))) {
if ((str = dict.FindString(opt))) {
aboutMessage.append(*str).append("\r\n");
}
}

if ((str = dict.FindStringKey("iconPath"))) {
if ((str = dict.FindString("iconPath"))) {
base::FilePath path = base::FilePath::FromUTF8Unsafe(*str);
electron::util::PopulateImageSkiaRepsFromPath(&image, path);
}
Expand All @@ -770,7 +769,7 @@ void Browser::ShowAboutPanel() {
}

void Browser::SetAboutPanelOptions(base::Value::Dict options) {
about_panel_options_ = base::Value(std::move(options));
about_panel_options_ = std::move(options);
}

} // namespace electron
9 changes: 9 additions & 0 deletions spec/api-app-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1859,6 +1859,15 @@ describe('app module', () => {
})).to.eventually.be.rejectedWith(/ERR_NAME_NOT_RESOLVED/);
});
});

describe('about panel', () => {
it('app.setAboutPanelOptions() does not crash', () => {
app.setAboutPanelOptions({
applicationName: 'electron!!',
version: '1.2.3'
});
});
});
});

describe('default behavior', () => {
Expand Down