Last Comment Bug 774130 - Implement desktop background on GNOME
: Implement desktop background on GNOME
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: OS Integration (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
: 215532 (view as bug list)
Depends on: 771941 773999
Blocks: 632585 779300
  Show dependency treegraph
 
Reported: 2012-07-15 16:39 PDT by neil@parkwaycc.co.uk
Modified: 2012-07-31 14:09 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Proposed patch (6.98 KB, patch)
2012-07-15 16:42 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
Details | Diff | Review
Addressed review comments (7.04 KB, patch)
2012-07-16 13:39 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
Details | Diff | Review

Description neil@parkwaycc.co.uk 2012-07-15 16:39:05 PDT
This is just the basic implementation that works with our existing context menuitem.
Comment 1 neil@parkwaycc.co.uk 2012-07-15 16:42:11 PDT
Created attachment 642447 [details] [diff] [review]
Proposed patch
Comment 2 Ian Neal 2012-07-16 11:52:22 PDT
Comment on attachment 642447 [details] [diff] [review]
Proposed patch

>+++ b/suite/shell/src/nsGNOMEShellService.cpp

>+#include "nsIStringBundle.h"
>+#include "nsIImageLoadingContent.h"
>+#include "nsIDOMElement.h"
Firefox seems to use "nsIDOMHTMLImageElement.h" here.

>+#include "imgIRequest.h"
>+#include "imgIContainer.h"
>+#include "nsIImageToPixbuf.h"
Firefox seems to #ifdef MOZ_WIDGET_GTK2 this.

>+  // Set desktop wallpaper filling style
>+  nsCAutoString options;
>+  if (aPosition == BACKGROUND_TILE)
>+    options.AssignLiteral("wallpaper");
>+  else if (aPosition == BACKGROUND_STRETCH)
>+    options.AssignLiteral("stretched");
>+  else if (aPosition == BACKGROUND_FILL)
>+    options.AssignLiteral("zoom");
>+  else if (aPosition == BACKGROUND_FIT)
>+    options.AssignLiteral("scaled");
>+  else
>+    options.AssignLiteral("centered");
Would using switch here be possible so it is similar to the Windows one?

>+    // Set the image to an empty string first to force a refresh
>+    // (since we could be writing a new image on top of an existing
>+    // Firefox_wallpaper.png and nautilus doesn't monitor the file for changes)
Do we really mean "Firefox_wallpaper.png" here?

r=me with those answered/addressed
Comment 3 neil@parkwaycc.co.uk 2012-07-16 12:44:22 PDT
(In reply to Ian Neal from comment #2)
> >+#include "nsIDOMElement.h"
> Firefox seems to use "nsIDOMHTMLImageElement.h" here.
We don't actually use the specific interface.

> >+#include "nsIImageToPixbuf.h"
> Firefox seems to #ifdef MOZ_WIDGET_GTK2 this.
We only build this file on GTK2 builds as per the Makefile.

> >+  // Set desktop wallpaper filling style
> Would using switch here be possible so it is similar to the Windows one?
Yeah, I noticed that when writing bug 773999; I'll change it.

> >+    // Firefox_wallpaper.png and nautilus doesn't monitor the file for changes)
> Do we really mean "Firefox_wallpaper.png" here?
Oops :-[
Comment 4 neil@parkwaycc.co.uk 2012-07-16 13:39:51 PDT
Created attachment 642711 [details] [diff] [review]
Addressed review comments
Comment 5 neil@parkwaycc.co.uk 2012-07-17 13:49:27 PDT
*** Bug 215532 has been marked as a duplicate of this bug. ***
Comment 6 neil@parkwaycc.co.uk 2012-07-17 13:50:11 PDT
Pushed comm-central changeset 84f1af072b63.
Comment 7 neil@parkwaycc.co.uk 2012-07-19 12:33:40 PDT
Oops, forgot to mark fixed at the same time...

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