Last Comment Bug 779300 - Implement desktop background colour on GNOME
: Implement desktop background colour on GNOME
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: OS Integration (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: seamonkey2.14
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on: 632585 774130
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-31 14:09 PDT by neil@parkwaycc.co.uk
Modified: 2012-08-05 09:50 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (4.37 KB, patch)
2012-07-31 14:13 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2012-07-31 14:09:34 PDT
Now that we have the new fancy desktop background dialog, we might as well support the colour picker.
Comment 1 neil@parkwaycc.co.uk 2012-07-31 14:13:03 PDT
Created attachment 647678 [details] [diff] [review]
Proposed patch
Comment 2 Ian Neal 2012-08-04 06:06:03 PDT
Comment on attachment 647678 [details] [diff] [review]
Proposed patch

>   // build the file name
>-  nsCAutoString filePath(PR_GetEnv("HOME"));
>+  nsCString filePath(PR_GetEnv("HOME"));
Part of another patch?

> NS_IMETHODIMP
> nsGNOMEShellService::GetDesktopBackgroundColor(PRUint32 *aColor)
> {

>+  if (background.IsEmpty())
>+    return NS_ERROR_FAILURE;
Firefox does *aColor = 0; return NS_OK here - should we be doing the same?
Comment 3 neil@parkwaycc.co.uk 2012-08-04 09:49:16 PDT
(In reply to Ian Neal from comment #2)
> (From update of attachment 647678 [details] [diff] [review])
> >-  nsCAutoString filePath(PR_GetEnv("HOME"));
> >+  nsCString filePath(PR_GetEnv("HOME"));
> Part of another patch?
No, just random cleanup (nsStringAPI typdefs nsCAutoString to nsCString, and this also helps us avoid the mass nsCAutoString -> nsAutoCString change that has been bandied about).

> >+  if (background.IsEmpty())
> >+    return NS_ERROR_FAILURE;
> Firefox does *aColor = 0; return NS_OK here - should we be doing the same?
The idea is that this will cause us to hide the colour picker, so that we don't try to change the background colour, since it seems likely that it would fail too.
Comment 4 Philip Chee 2012-08-05 09:50:56 PDT
Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/872744c69ef8

Note You need to log in before you can comment on or make changes to this bug.