Last Comment Bug 276299 - "Mozilla Wallpaper.bmp" should be localizable.
: "Mozilla Wallpaper.bmp" should be localizable.
: fixed-seamonkey1.0, fixed1.8, l12y
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- minor (vote)
: ---
Assigned To: Vidar Haarr (not reading bugmail)
Depends on: 260437
  Show dependency treegraph
Reported: 2004-12-28 18:05 PST by O. Atsushi (Torisugari)
Modified: 2008-07-31 04:22 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch v1 [suite, windows] (maybe already obsolete) (3.16 KB, patch)
2004-12-28 22:18 PST, O. Atsushi (Torisugari)
no flags Details | Diff | Splinter Review
patch v2 (3.17 KB, patch)
2005-09-08 08:46 PDT, Vidar Haarr (not reading bugmail)
neil: review+
Details | Diff | Splinter Review
version 2.1 (3.67 KB, patch)
2005-10-08 03:42 PDT, Vidar Haarr (not reading bugmail)
vhaarr+bmo: review+
jag-mozilla: superreview+
Details | Diff | Splinter Review
patch for checkin, I hope. (4.10 KB, patch)
2005-10-09 00:42 PDT, Vidar Haarr (not reading bugmail)
vhaarr+bmo: review+
vhaarr+bmo: superreview+
kairo: approval‑seamonkey1.0+
Details | Diff | Splinter Review
adds localization notes (894 bytes, patch)
2005-10-10 15:47 PDT, Vidar Haarr (not reading bugmail)
kairo: review+
jag-mozilla: superreview+
Details | Diff | Splinter Review

Description O. Atsushi (Torisugari) 2004-12-28 18:05:52 PST
According to my desktop property, all file names but "Mozilla Wallpaper" are
written in
Japanese. IIRC, default wallpapers in Windows95 were also in Japanese. I don't know
much about MS GUI guidelines, but it's nice to remove this hard coding..
Comment 1 O. Atsushi (Torisugari) 2004-12-28 22:18:45 PST
Created attachment 169797 [details] [diff] [review]
patch v1 [suite, windows] (maybe already obsolete)

Is this OS=ALL bug?
Hmm, it seems more investigation needed...
Comment 3 Vidar Haarr (not reading bugmail) 2005-02-03 02:20:48 PST
Torisugari: Does this patch need to be updated to trunk, or is it ready for review ?
Comment 4 O. Atsushi (Torisugari) 2005-03-01 01:19:45 PST
(In reply to comment #3)
Sorry for the long absence.

> Does this patch need to be updated to trunk,
> or is it ready for review ?

Well, imho, this patch is 99% ready.

Only one thing:
The locale data enty of the patch is located in
However, if we are going to implement
this wall paper image functionality on linux
: bug 215532 (and mac and so on as well), may not be the proper
file. And in that case, I'm not sure which file I
should move the entry to, at all.

Anybody any idea/info?
Comment 5 Vidar Haarr (not reading bugmail) 2005-03-04 00:32:31 PST
I asked because I wanted you to mark it "review?" from for example Dean or Neil :)
Comment 6 2005-03-05 13:58:50 PST
Comment on attachment 169797 [details] [diff] [review]
patch v1 [suite, windows] (maybe already obsolete)

>+  nsXPIDLString brandName, fileLeaf;
You're allowed long variable names, you know... fileLeafName is much more

>   nsCOMPtr<nsIStringBundleService> bundleService(do_GetService(bundleCID));
You should probably use the 2-arg form of do_GetService so you can return the
correct rv on failure.
Comment 7 Vidar Haarr (not reading bugmail) 2005-09-08 08:46:47 PDT
Created attachment 195277 [details] [diff] [review]
patch v2

Patch updated to trunk and fixes issues mentioned in comment #6.

Please note that I do not have a Windows environment and can't really test this
patch, but it is only attachment 169797 [details] [diff] [review] updated to trunk
(chrome://global/locale/ is now
chrome://branding/locale/ and two trivial changes, so it
should be OK, provided tested his original patch. :-)
Comment 8 Vidar Haarr (not reading bugmail) 2005-10-06 04:30:25 PDT
Mano: Any thoughts on what to do about the various implementations that exist in
toolkit for setting desktop background?

The difference in the nsShellService::SetDesktopBackground implementations for
the different platforms are crazy (toolkit).

For example, for the filename, Windows uses a .properties string, Mac uses the
image filename and GNOME uses $brandname_wallpaper.png.

the mac impl also supports setting images that are not fully loaded yet as the
background because it actually fires off a new SaveURI and implements
nsIWebProgressListener, and it fires off a global notification
"shell:desktop-background-changed" when it actually sets the background in
Comment 9 2005-10-07 06:15:27 PDT
Comment on attachment 195277 [details] [diff] [review]
patch v2

>+  const PRUnichar* stringArray = brandName.get();
My superreview hat tells me that a PRUnichar* is not a string array. You could
rename it, or you could tweak the code so that it is actually an array (in
which case use NS_ARRAY_LENGTH instead of 1).
Comment 10 Vidar Haarr (not reading bugmail) 2005-10-08 03:42:57 PDT
Created attachment 198911 [details] [diff] [review]
version 2.1

Thank you, Neil.

Please note, again, that I have no means of testing this patch, so reviewers
should take special care and even check for compile errors/warnings.
Comment 11 jag (Peter Annema) 2005-10-08 04:53:45 PDT
Comment on attachment 198911 [details] [diff] [review]
version 2.1

Code looks good, just a minor nit to pick.

>Index: xpfe/components/winhooks/nsWindowsHooks.cpp

>+  nsCOMPtr<nsIStringBundleService> bundleService(do_GetService(bundleCID, &rv));

Just get rid of the nsCID stuff and use the CONTRACTID instead:

bundleService(do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv));

or if you care about the 80 columns style rule:

  nsCOMPtr<nsIStringBundleService> bundleService =

I don't have a Windows tree I can build right now, let alone test this patch,
so if you make that change please have whoever's gonna check this in do a test
build first :-)

Comment 12 Vidar Haarr (not reading bugmail) 2005-10-09 00:42:56 PDT
Created attachment 198982 [details] [diff] [review]
patch for checkin, I hope.

The nsCID has a brother, so I killed him too.

This file doesn't care very much about wrapping on 80, but it may look like it
cares sometimes. Personally, I don't care.
Comment 13 2005-10-09 13:32:23 PDT
Fix checked in.
Comment 14 Christian :Biesinger (don't email me, ping me on IRC) 2005-10-09 16:47:12 PDT
Should the ".bmp" part really be localizable?

Also, shouldn't there be a localization note about what the %S means?
Comment 15 O. Atsushi (Torisugari) 2005-10-09 19:26:22 PDT
I agree about comment 14, so feel free to reopen.

-> verified
Comment 16 jag (Peter Annema) 2005-10-10 10:21:01 PDT
Good call, biesi. The .bmp part doesn't need to be localizable and probably
shouldn't be.

Vidar, would you mind addressing that, and adding a quick note to to explain what the %S there stands for?
Comment 17 Vidar Haarr (not reading bugmail) 2005-10-10 15:47:06 PDT
Created attachment 199110 [details] [diff] [review]
adds localization notes

Adds a localization note to wallpaperFile and promptText. Neil had already done
the ".bmp" appending before checking in attachment 198982 [details] [diff] [review].
Comment 18 Robert Kaiser 2005-10-11 04:19:28 PDT
Comment on attachment 199110 [details] [diff] [review]
adds localization notes

Thos L10n note look good to me ;-)
Comment 19 Vidar Haarr (not reading bugmail) 2005-10-11 14:51:43 PDT
Comment on attachment 199110 [details] [diff] [review]
adds localization notes

Thanks for the reviews! If someone could help me check this in, that would be
Comment 20 Robert Kaiser 2005-10-12 16:48:51 PDT
L10n notes checked in as well, thanks!
Comment 21 2005-12-09 09:50:55 PST
Comment on attachment 198982 [details] [diff] [review]
patch for checkin, I hope.

(including the L10N notes of course)
Comment 22 Robert Kaiser 2005-12-09 15:48:40 PST
Comment on attachment 198982 [details] [diff] [review]
patch for checkin, I hope.

a=me given you check it in with the same modifications as done on trunk
Comment 23 Martijn Wargers [:mwargers] (not working for Mozilla) 2005-12-09 16:57:01 PST
Please advise if you want me to provide a patch for branch checkin.
Comment 24 Vidar Haarr (not reading bugmail) 2005-12-09 16:58:00 PST
Err.. Comment 23 was mine. Yay for public access computers.
Comment 25 2005-12-11 13:02:45 PST
(In reply to comment #23)
>Please advise if you want me to provide a patch for branch checkin.
This case was particularly easy as there were no other changes on trunk.

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