296583 - Make "Set Desktop Background" dialog skinable

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P2)

Description

[Jeff Walden [:Waldo]]

mozilla/browser/base/content/monitor.png is used in the Set Wallpaper dialog
(and only there; LXR for monitor.png to prove it).  It's in content, so it's not
skinnable.  I'm not sure why it's not skinnable, and I can't think of a good
reason why it shouldn't be.

I can try making a patch after the l10n freeze, but I won't have time before then.

Comment 1

[Mike Connor [:mconnor]]

Myself, I can't think of a reason it should be skinnable, and changing the size
could seriously affect the dialog.  There's a nice little problem though... this
looks like it was lifted straight from Windows XP....

Comment 2

[Jeff Walden [:Waldo]]

Just as an update for those not following every checkin, the code at the
following link was recently checked into source.  A comment there explains why
this will need to be skinnable.

http://lxr.mozilla.org/mozilla/source/browser/components/shell/content/setDesktopBackground.xul#98

Comment 3

[Kevin Gerich]

Attachment: new monitor.png in action

<!-- XXXben - this needs to be skinable - on MacOSX we want to draw a
	      bucket, not a monitor -->

I don't agree that we want to draw an image well, because that implies that you
can drop images on it. Here's a Mac-like monitor.png. Think this would work
crossplatform?

Comment 4

[Kevin Gerich]

Attachment: Here's a drop-in replacement for monitor.png. Same resolution and everything. This is a bit mac-looking but also a bit less stolen than the current image.

Comment 5

[Asaf Romano (gone)]

Please file a follow-up on replacing the images.

Comment 6

[Asaf Romano (gone)]

Attachment: patch

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 245700 [details] [diff] [review]
patch

>Index: browser/themes/pinstripe/browser/setDesktopBackground.css

>+@namespace url("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul");
>+
>+#monitorImage {
>+  list-style-image: url("chrome://browser/skin/monitor.png");
>+}

>Index: browser/themes/winstripe/browser/setDesktopBackground.css

>+#noPreviewAvailable {
>+  background-color: white !important;
>+  font-size: 12px !important;
>+}

Does this rule not belong in pinstripe? It used to apply there as well, afaict.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

And I think we should probably just replace the Mac image with attachment 200153 [details] (maybe beltzner can UI-R+ it!)

Comment 9

[Asaf Romano (gone)]

BACKGROUND_TILE is not implemented, thus the noPreviewAvailable element is never added to the DOM.

Comment 10

[Asaf Romano (gone)]

mozilla/browser/base/jar.mn 1.110
mozilla/browser/base/content/browser.css 1.22
mozilla/browser/base/content/monitor.png delete
mozilla/browser/components/shell/content/setDesktopBackground.xul 1.3
mozilla/browser/themes/pinstripe/browser/jar.mn 1.35
mozilla/browser/themes/pinstripe/browser/monitor.png initial revision: 1.1
mozilla/browser/themes/pinstripe/browser/setDesktopBackground.css initial revision: 1.1
mozilla/browser/themes/winstripe/browser/jar.mn 1.32
mozilla/browser/themes/winstripe/browser/monitor.png initial revision: 1.1
mozilla/browser/themes/winstripe/browser/setDesktopBackground.css initial revision: 1.1

Comment 11

[Asaf Romano (gone)]

Checked in attachment 200153 [details] as well, thanks Kevin
mozilla/browser/themes/pinstripe/browser/monitor.png 1.2

Comment 12

[Jeff Walden [:Waldo]]

(In reply to comment #5)
> Please file a follow-up on replacing the images.

Bug 362239 filed to replace the Windows image.

Comment 13

[Dão Gottwald [:dao]]

(In reply to comment #4)
> Created an attachment (id=200153) [edit]
> Here's a drop-in replacement for monitor.png. Same resolution and everything.
> This is a bit mac-looking but also a bit less stolen than the current image. 

There's a one-pixel artifact on the left.

Comment 14

[Dão Gottwald [:dao]]

(In reply to comment #13)
> (In reply to comment #4)
> > Created an attachment (id=200153) [edit]
> > Here's a drop-in replacement for monitor.png. Same resolution and everything.
> > This is a bit mac-looking but also a bit less stolen than the current image. 
> 
> There's a one-pixel artifact on the left.

Errr, the other left, like: on the right ... bottom right.

Comment 15

[timeless]

in case someone named "magento" comes around to comment in this bug. this isn't the bug he's looking for. had he stuck around on irc long enough there was a long and detailed explanation as to why this isn't the bug he's looking for. the one he wants is Bug 274374.