GNOME Bugzilla – Bug 330355
Mailer Preferences. New mail notification, "specify filename" should be disabled
Last modified: 2013-09-13 00:48:12 UTC
Under Mailer preferences, in "New mail notifications" section, the "specify filename" widget should be enabled only when the "play sound file when a new mail arrives" option is selected. Other information:
Confirming.
Created attachment 63667 [details] [review] mailer preferences.New mail notification, "specific filename" entry is enabled only when "play sound file when new mail arrives" button is selected
I dont see why you use fonts/use_custom. really a gconf key might not be reqd at all. Just enable/disable the field based on the selected radio button.
Created attachment 68530 [details] [review] Patch for disabling "specify filename" in new mail notification..and enabling it only for "Play sound file when new mail arrives" option
hi raghav, your patch is not a clean one - please take a look at it! please provide a clean patch that only includes code changes that are really needed to fix this bug, your current patch includes stuff which also alters codelines (means: you have added unnecessary whitespace a few times and deleted one whitespace) that should not be touched to fix this problem. thanks a lot. :-)
Created attachment 68714 [details] [review] Patch for disabling "specify filename" in new mail notification..and enabling it only for "Play sound file when new mail arrives" option
raghav: - ChangeLog: "specify filename" was *en*abled for radNotifyNot and radNotifyBeep, not *disabled*. :-) - your patch is still not clean... there is one whitespace in front of gtk_widget_set_sensitive ((GtkWidget *) prefs->notify_sound_file, TRUE); which should not be there. yes, i can be pedantic. - any reason why you use if ((val == MAIL_CONFIG_NOTIFY_NOT) || (val == MAIL_CONFIG_NOTIFY_BEEP)) instead of if (val != MAIL_CONFIG_NOTIFY_PLAY_SOUND) ? (just curious.) apart from that, patch works. :-)
Created attachment 68939 [details] [review] pimped version no idea; no offense.
raghav: slap me, seems like the additional whitespace isn't your fault :-)
Fixed to HEAD
please see my comments to bug 350411