Closed
Bug 473337
Opened 15 years ago
Closed 15 years ago
Refresh the updater application icon
Categories
(Toolkit :: Application Update, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.9.2a1
People
(Reporter: faaborg, Assigned: ddahl)
References
()
Details
(Keywords: polish, verified1.9.1, Whiteboard: [polish-easy] [polish-visual] [icon-shiretoko][icon-refresh][polish-p2])
Attachments
(7 files, 13 obsolete files)
39.08 KB,
application/x-zip-compressed
|
Details | |
7.23 KB,
image/png
|
faaborg
:
ui-review+
|
Details |
11.21 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
90.68 KB,
application/octet-stream
|
Details | |
54.66 KB,
application/octet-stream
|
Details | |
39.21 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
39.96 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
This bug is for landing new icons for software update on all platforms. I'll attach the new files. See http://blog.mozilla.com/faaborg/2008/12/16/new-os-x-icons-made-by-sofa/ for discussion on the design work.
Reporter | ||
Updated•15 years ago
|
Whiteboard: [polish-easy] [polish-visual] [icon-3.1]
Reporter | ||
Comment 1•15 years ago
|
||
This file should replace the file at /toolkit/mozapps/update/src/updater/macbuild/Contents/Resources/updater.icns I'm likely going to post a new file that only goes up to 128x128 in resolution since this icon only appears in the dock (and that is the maximum size for dock icons). However, we should go ahead and land this icon for beta 3 and we can reduce the file size in a follow up bug.
Reporter | ||
Comment 2•15 years ago
|
||
This file should replace the file at /toolkit/mozapps/update/src/updater/updater.ico
Reporter | ||
Comment 3•15 years ago
|
||
Similar to crash reporter, I don't think linux currently has an updater icon. Here is the source artwork as png files. We can either land the new linux icon as part of this bug, or as a follow up bug.
Reporter | ||
Updated•15 years ago
|
Keywords: checkin-needed
Reporter | ||
Updated•15 years ago
|
Attachment #356672 -
Flags: ui-review+
Reporter | ||
Updated•15 years ago
|
Attachment #356673 -
Flags: ui-review+
Comment 4•15 years ago
|
||
I'll take care of the windows and mac icons
Assignee: nobody → robert.bugzilla
Assignee | ||
Comment 5•15 years ago
|
||
I will take care of the Linux icons
Updated•15 years ago
|
Attachment #356673 -
Flags: ui-review+ → ui-review-
Comment 6•15 years ago
|
||
Comment on attachment 356673 [details]
New Updater icon for Windows
Talked with Alex about this icon and it is missing several resolutions so r-'ing so it doesn't accidentally get checked in.
Updated•15 years ago
|
Keywords: checkin-needed
Updated•15 years ago
|
Assignee: robert.bugzilla → nobody
Reporter | ||
Comment 7•15 years ago
|
||
Let's try to check in the OS X icon in for beta 3 if possible.
Flags: blocking1.9.1?
Comment 8•15 years ago
|
||
Alex - there are ten images in the current image file checked in for mac, and only five in the new one. Is this a problem?
Comment 9•15 years ago
|
||
Alex, I noticed that the new Mac image is significantly smaller than the existing one... the existing image has fallback to lower resolutions the same as the Windows one needs to have. Are you sure you want to go with just the high resolution images on Mac?
Assignee | ||
Comment 10•15 years ago
|
||
Comment on attachment 356674 [details]
Updater icon for Linux
inside of the updater code is this comment:
// GTK 2.2 seems unable to prevent our dialog from being closed when
// the user hits the close button on the dialog. This problem only
// occurs when either one of the following methods are called:
// gtk_window_set_position
// gtk_window_set_type_hint
// For this reason, we disable window decorations.
So no window frame means no icon. Any pointers?
Attachment #356674 -
Flags: review?(gavin.sharp)
Reporter | ||
Comment 11•15 years ago
|
||
>Alex - there are ten images in the current image file checked in for mac, and >only five in the new one. Is this a problem? >Alex, I noticed that the new Mac image is significantly smaller than the >existing one... the existing image has fallback to lower resolutions the same >as the Windows one needs to have. Are you sure you want to go with just the >high resolution images on Mac? I just passed the file off assuming our design firm was making the correct decisions. I'll need to look into this more carefully to figure out exactly why there was a change (possibly related to which OS versions we are intending to support on OS X).
Comment 12•15 years ago
|
||
(In reply to comment #10) Looks like GTK 2.2 is beneath the system requirements for Fx3.0, http://www.mozilla.com/en-US/firefox/system-requirements.html so that comment probably doesn't apply anymore - assuming the whole GTK 2.2 vs 2.20 doesn't apply.
Updated•15 years ago
|
Attachment #356674 -
Flags: review?(gavin.sharp)
Comment 13•15 years ago
|
||
Comment on attachment 356674 [details]
Updater icon for Linux
I'm not really sure what I'm supposed to be reviewing here - icons look fine to me, but I don't know enough about the updater or GTK to be able to offer advice about how they should be added.
Assignee | ||
Comment 14•15 years ago
|
||
Thanks Nick. I'll do some more research on this GTK stuff.
Updated•15 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Comment 15•15 years ago
|
||
Who owns this? David, are you actively working on this? This blocker needs an owner.
Comment 16•15 years ago
|
||
Right now the owner is alex to address 6 and comment 11.
Assignee: nobody → faaborg
Assignee | ||
Comment 17•15 years ago
|
||
I'll own it after alex gets the icons for mac and win.
Assignee: faaborg → ddahl
Assignee | ||
Comment 18•15 years ago
|
||
Just wanted to get some review mid-way. The updater UI now has the new icon, an actual frame and titlebar, etc... This is not done yet, RS and I will work on the rest of it today.
Comment 19•15 years ago
|
||
David, this is the file that will need to copy the image used by the updater binary. http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsUpdateDriver.cpp
Assignee | ||
Comment 20•15 years ago
|
||
I am using the 48x48 png for this. The icon shows up in the window frame and alt-tab switcher. I will post another diff of code once I clean it up.
Attachment #359838 -
Flags: ui-review?(faaborg)
Assignee | ||
Comment 21•15 years ago
|
||
Whoops - window was not focused. Now it is.
Attachment #359838 -
Attachment is obsolete: true
Attachment #359841 -
Flags: ui-review?(faaborg)
Attachment #359838 -
Flags: ui-review?(faaborg)
Assignee | ||
Comment 22•15 years ago
|
||
I am adding this patch to show how I tested everything. Rob Strong helped me figure out how to shirt circuit the updater just to see the UI. I will remove the short circuiting comments before posting a patch for review.
Assignee | ||
Comment 23•15 years ago
|
||
Will confer with RS on monday about this patch. I think there are some issues wiht the Makefile, also need additional linux and other platforms to test. Not asking for review yet.
Assignee | ||
Updated•15 years ago
|
Attachment #359533 -
Attachment is obsolete: true
Assignee | ||
Comment 24•15 years ago
|
||
This is a helper script to speed up testing withe the above "test patch"
Reporter | ||
Comment 25•15 years ago
|
||
Comment on attachment 359841 [details]
Linux updater window w/ Icon and Frame
What do you mean when you say you are using the 48x48 icon for this? Is that to deal with cases where the title bar is taller and an icon larger than 16x16 is required? I think the smaller sized icons are pixel polished, but this doesn't seem to look particularly bad.
Assignee | ||
Comment 26•15 years ago
|
||
I picked the largest size because the alt-tab application icon is bigger, but now that I think about it, it is probably only about 24 x 24.
Assignee | ||
Comment 27•15 years ago
|
||
I have gotten the icon appear in a framed gtk window. When I build a package and test by gaming the app.update.url.override with a recent build update url, the updater does not display the icon. When the progressgui_gtk ShowProgressUI() tries to free the pixbuf, I get this error: (updater:22452): GLib-GObject-CRITICAL **: g_object_unref: assertion `G_IS_OBJECT (object)' failed So the icon is never copied into the $DIST/updates/0/ directory. I do not understand the build system, so thought I would ping Ted on this. (sdwilsh told me to:))
Attachment #359845 -
Attachment is obsolete: true
Attachment #359851 -
Attachment is obsolete: true
Attachment #359854 -
Attachment is obsolete: true
Attachment #360409 -
Flags: review?(ted.mielczarek)
Comment 28•15 years ago
|
||
(In reply to comment #27) > Created an attachment (id=360409) [details] > working, but has build issues > > I have gotten the icon appear in a framed gtk window. > > When I build a package and test by gaming the app.update.url.override with a > recent build update url, the updater does not display the icon. When the > progressgui_gtk ShowProgressUI() tries to free the pixbuf, I get this error: > (updater:22452): GLib-GObject-CRITICAL **: g_object_unref: assertion > `G_IS_OBJECT (object)' failed > > So the icon is never copied into the $DIST/updates/0/ directory. > > I do not understand the build system, so thought I would ping Ted on this. > (sdwilsh told me to:)) David, the png is copied into updates/0/ in nsUpdateDriver.cpp during runtime so this isn't a build problem
Assignee | ||
Comment 29•15 years ago
|
||
I don't see the png in my updates/0/ dir. is it removed right away?
Comment 30•15 years ago
|
||
(In reply to comment #29) > I don't see the png in my updates/0/ dir. is it removed right away? It is removed immediately after the update is applied during the restart just as the updater binary, etc. are removed. Also, I suspect the png isn't being copied successfully to updates/0/ by nsUpdateDriver.cpp
Assignee | ||
Comment 31•15 years ago
|
||
Looks like it works now. (I rebuilt the entire tree). Sorry for the noise, I guess I need to figure out everything that needs to be built before reporting a problem. Attaching a new patch asap.
Assignee | ||
Comment 32•15 years ago
|
||
Attachment #360409 -
Attachment is obsolete: true
Attachment #360409 -
Flags: review?(ted.mielczarek)
Assignee | ||
Comment 33•15 years ago
|
||
According to this document, 48x48 is the minimal default icon to include with an application. Of course this is more for dealing with display of documents via mime-types in Nautilus or Dolphin. Also, KDE4 tends to have much larger icons when you hit alt-tab. The default size on Gnome when hitting alt-tab is 32x32 px. I think we should leave it at 48x48 as it scales down well, and will scale up better than smaller icons.
Assignee | ||
Updated•15 years ago
|
Attachment #360543 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 34•15 years ago
|
||
Forgot to post the Gnome icon spec: http://library.gnome.org/devel/icon-theme-spec/
Updated•15 years ago
|
Attachment #360543 -
Flags: review?(robert.bugzilla) → review-
Comment 35•15 years ago
|
||
Comment on attachment 360543 [details] [diff] [review] Will review with rob strong >diff --git a/browser/installer/unix/packages-static b/browser/installer/unix/packages-static >--- a/browser/installer/unix/packages-static >+++ b/browser/installer/unix/packages-static >@@ -276,16 +276,17 @@ bin/chrome/icons/default/default32.png > bin/chrome/icons/default/default48.png > bin/chrome/reporter.manifest > bin/chrome/reporter.jar > bin/@PREF_DIR@/reporter.js > > ; shell icons > bin/icons/*.xpm > bin/icons/*.png >+bin/icons/updater.png Not necessary since the line above the added line includes all png's in the icons directory. >diff --git a/toolkit/mozapps/update/src/updater/progressui_gtk.cpp b/toolkit/mozapps/update/src/updater/progressui_gtk.cpp >--- a/toolkit/mozapps/update/src/updater/progressui_gtk.cpp >+++ b/toolkit/mozapps/update/src/updater/progressui_gtk.cpp >@@ -16,16 +16,17 @@ > * The Original Code is mozilla.org code. > * > * The Initial Developer of the Original Code is Google Inc. > * Portions created by the Initial Developer are Copyright (C) 2005 > * the Initial Developer. All Rights Reserved. > * > * Contributor(s): > * Darin Fisher <darin@meer.net> >+ * David Dahl <ddahl@mozilla.com> > * > * Alternatively, the contents of this file may be used under the terms of > * either the GNU General Public License Version 2 or later (the "GPL"), or > * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), > * in which case the provisions of the GPL or the LGPL are applicable instead > * of those above. If you wish to allow use of your version of this file only > * under the terms of either the GPL or the LGPL, and not to allow others to > * use your version of this file under the terms of the MPL, indicate your >@@ -48,22 +49,23 @@ > static float sProgressVal; // between 0 and 100 > static gboolean sQuit = FALSE; > static gboolean sEnableUI; > static guint sTimerID; > > static GtkWidget *sWin; > static GtkWidget *sLabel; > static GtkWidget *sProgressBar; >- Leave this newline you removed >+static GdkPixbuf *pixbuf; Make this local to the function > static const char *sProgramPath; > > static gboolean > UpdateDialog(gpointer data) > { >+ Remove the newline you added > if (sQuit) > { > gtk_widget_hide(sWin); > gtk_main_quit(); > } > > float progress = sProgressVal; > >@@ -78,17 +80,16 @@ OnDeleteEvent(GtkWidget *widget, GdkEven > { > return TRUE; > } > > int > InitProgressUI(int *pargc, char ***pargv) > { > sProgramPath = (*pargv)[0]; >- Leave this newline you removed > sEnableUI = gtk_init_check(pargc, pargv); > return 0; > } > > int > ShowProgressUI() > { > if (!sEnableUI) >@@ -97,42 +98,43 @@ ShowProgressUI() > // Only show the Progress UI if the process is taking significant time. > // Here we measure significant time as taking more than one second. > > usleep(500000); > > if (sQuit || sProgressVal > 50.0f) > return 0; > >- char path[PATH_MAX]; >- snprintf(path, sizeof(path), "%s.ini", sProgramPath); >- >+ char ini_path[PATH_MAX]; >+ snprintf(ini_path, sizeof(ini_path), "%s.ini", sProgramPath); >+ Remove the spaces from the empty line > StringTable strings; >- if (ReadStrings(path, &strings) != OK) >+ if (ReadStrings(ini_path, &strings) != OK) > return -1; > > sWin = gtk_window_new(GTK_WINDOW_TOPLEVEL); > if (!sWin) > return -1; > >- // GTK 2.2 seems unable to prevent our dialog from being closed when >- // the user hits the close button on the dialog. This problem only >- // occurs when either one of the following methods are called: >- // gtk_window_set_position >- // gtk_window_set_type_hint >- // For this reason, we disable window decorations. >- >+ char icon_path[PATH_MAX]; >+ snprintf(icon_path, sizeof(icon_path), "%s.png", sProgramPath); >+ > g_signal_connect(G_OBJECT(sWin), "delete_event", > G_CALLBACK(OnDeleteEvent), NULL); >- Leave this newline you removed > gtk_window_set_title(GTK_WINDOW(sWin), strings.title); > gtk_window_set_type_hint(GTK_WINDOW(sWin), GDK_WINDOW_TYPE_HINT_DIALOG); > gtk_window_set_position(GTK_WINDOW(sWin), GTK_WIN_POS_CENTER_ALWAYS); > gtk_window_set_resizable(GTK_WINDOW(sWin), FALSE); >- gtk_window_set_decorated(GTK_WINDOW(sWin), FALSE); >+ gtk_window_set_decorated(GTK_WINDOW(sWin), TRUE); >+ Remove this empty line
Assignee | ||
Comment 36•15 years ago
|
||
cleaned it up
Attachment #360543 -
Attachment is obsolete: true
Attachment #360612 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 37•15 years ago
|
||
Attachment #360612 -
Attachment is obsolete: true
Attachment #360613 -
Flags: review?(robert.bugzilla)
Attachment #360612 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 38•15 years ago
|
||
Attachment #360613 -
Attachment is obsolete: true
Attachment #360618 -
Flags: review?(robert.bugzilla)
Attachment #360613 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 39•15 years ago
|
||
Comment on attachment 360618 [details] [diff] [review] formatting clean up, fixed Makefile with SYSINSTALL Ted: Just want to make sure the Makefile is ok here - we need i tot copy the png into dist/bin/icons thanks! ddahl
Attachment #360618 -
Flags: review?(ted.mielczarek)
Reporter | ||
Updated•15 years ago
|
Attachment #359841 -
Flags: ui-review?(faaborg) → ui-review+
Updated•15 years ago
|
Attachment #360618 -
Flags: review?(robert.bugzilla) → review+
Comment 40•15 years ago
|
||
Comment on attachment 360618 [details] [diff] [review] formatting clean up, fixed Makefile with SYSINSTALL Looks fine to me as long as Ted is cool with the Makefile changes
Comment 41•15 years ago
|
||
I believe these changes for Linux will also fix bug 302949... adding dependency
Blocks: 302949
Comment 42•15 years ago
|
||
Comment on attachment 360618 [details] [diff] [review] formatting clean up, fixed Makefile with SYSINSTALL +ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2) +libs:: updater.png + $(SYSINSTALL) $(IFLAGS1) $^ $(DIST)/bin/icons +endif I don't think you really need to list updater.png as a dependency here, it's not a generated file. Just hardcode updater.png in the INSTALL line. Also, you should use $(INSTALL), not $(SYSINSTALL). (SYSINSTALL is for installing outside of the objdir, here you want to use a symlink if possible.)
Comment 43•15 years ago
|
||
Comment on attachment 360618 [details] [diff] [review] formatting clean up, fixed Makefile with SYSINSTALL r+ with those changes.
Attachment #360618 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 44•15 years ago
|
||
cleaned up Makefile
Attachment #360618 -
Attachment is obsolete: true
Attachment #360967 -
Flags: review?(ted.mielczarek)
Comment 45•15 years ago
|
||
Comment on attachment 360967 [details] [diff] [review] changed sysinstall to install as per ted's advice Generally, if someone says "r+, with comments addressed", you don't need to re-request review. Just FYI.
Attachment #360967 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 46•15 years ago
|
||
Comment on attachment 360967 [details] [diff] [review] changed sysinstall to install as per ted's advice This is the patch to check in
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 47•15 years ago
|
||
before check in - rs recommends that we check Thunderbird, Sunbird and Seamonkey to make sure that bin/icons/*.png is in unix/packages-static Do all of the other platform packages source this information from here? http://mxr.mozilla.org/comm-central/source/mozilla/browser/installer/unix/packages-static#282
Comment 48•15 years ago
|
||
Thunderbird and Sunbird don't use a package manifest on Linux, only Windows; SeaMonkey's Linux manifest is http://mxr.mozilla.org/comm-central/source/suite/installer/unix/packages
Assignee | ||
Comment 49•15 years ago
|
||
How can Thunderbird and Sunbird take advantage of this patch? Do I need to add anything else to the Makefile, etc? (speaking out of complete ignorance here)
Comment 50•15 years ago
|
||
For a product/platform combination that defines a MOZ_PKG_MANIFEST (package-static or packages), the things listed in it are copied to the staging directory; for a product/platform that doesn't, dist/bin/* is tarred up and untarred in the staging directory. Then things listed in NO_PKG_FILES are removed, and what's left is what ships, so all you have to worry about is 1. Get into dist/bin 2. Be listed in any MOZ_PKG_MANIFEST_P (the before-preprocessing define for MOZ_PKG_MANIFEST, where you'll see separate platforms) that's defined for the platform you're interested in. 3. Don't match anything in NO_PKG_FILES (unlikely that you would, but just for completeness: don't ever name anything manglefoo and expect to be able to ship it out of dist/bin/manglefoo)
Assignee | ||
Comment 51•15 years ago
|
||
I had to rebuild my entire tree because of some unrelated tests, and got this error during Makefile creation: creating toolkit/mozapps/update/src/updater/Makefile make[8]: Entering directory `/home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/toolkit/mozapps/update/src/updater' Makefile:93: *** commands commence before first target. Stop. here is the offending bit: ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2) $(INSTALL) $(IFLAGS1) updater.png $(DIST)/bin/icons endif
Assignee | ||
Comment 52•15 years ago
|
||
Sorry for the trouble ted, I am a make newbie. This patch appears to work.
Attachment #360967 -
Attachment is obsolete: true
Attachment #361880 -
Flags: review?(ted.mielczarek)
Updated•15 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 53•15 years ago
|
||
Here is the windows icon with 8 bit and 4 bit versions
Attachment #356673 -
Attachment is obsolete: true
Reporter | ||
Comment 54•15 years ago
|
||
New icon for OS X (only goes up to 128x128)
Attachment #356672 -
Attachment is obsolete: true
Reporter | ||
Comment 55•15 years ago
|
||
New icons are ready to go (previous two comments), please let me know if anyone notices anything wrong with the new files.
Comment 56•15 years ago
|
||
Would it be possible to get a 256x256 one for Windows? Look under the Look and feel section on the following page. http://msdn.microsoft.com/en-us/library/aa511296.aspx
Updated•15 years ago
|
Whiteboard: [polish-easy] [polish-visual] [icon-3.1] → [polish-easy] [polish-visual] [icon-3.1][waiting ddahl]
Assignee | ||
Comment 57•15 years ago
|
||
Needs testing on a mac. Build the updater, make the package, check for updates and run the updater.
Reporter | ||
Comment 58•15 years ago
|
||
Is it possible to view the icon at 256x256? We have the artwork but I was just trying to keep the file size down.
Comment 59•15 years ago
|
||
Per the article... "Windows Vista-style icons scale smoothly between 256x256 and 32x32 pixels. The 256x256 pixel icon size is required to support high-resolution monitors." so it would appear it is used for scaling. Also, other-licenses/branding/firefox/firefox.ico has a 256x256 icon. Since the original didn't have a 256x256 we can just go with this one if you would prefer.
Comment 60•15 years ago
|
||
Comment on attachment 361880 [details] [diff] [review] I misunderstood ted's directions, build was failing. see comment 51 [Checkin: Comment 66] Any reason you reverted to listing updater.png as a dependency of the rule, which I asked you to change before?
Attachment #361880 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 61•15 years ago
|
||
> Any reason you reverted to listing updater.png as a dependency of the rule, > which I asked you to change before? make was dying with an error, see comment 51. I am sure Rob Strong knows why, I am somewhat of a make noob. Sorry.
Comment 62•15 years ago
|
||
Uh, you left out the target completely in that version, you want: +ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2) +libs:: + $(INSTALL) $(IFLAGS1) updater.png $(DIST)/bin/icons +endif
Assignee | ||
Comment 63•15 years ago
|
||
When I use your snippet in comment 62, this happens: c++ -o updater -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -pedantic -fno-strict-aliasing -fshort-wchar -pthread -pipe -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/libpng12 -I/usr/include/gtk-unix-print-2.0 -DNDEBUG -DTRIMMED -Os -freorder-blocks -fno-reorder-functions updater.o bspatch.o archivereader.o progressui_gtk.o readstrings.o -lpthread -Wl,-rpath-link,/home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/dist/bin -Wl,-rpath-link,/usr/local/lib -L../../../../../dist/bin -L../../../../../dist/lib ../../../../../modules/libmar/src/libmar.a -L../../../../../modules/libbz2/src -lbz2 -lasound -ldl -lm -lgtk-x11-2.0 -latk-1.0 -lpangoft2-1.0 -lfreetype -lz -lfontconfig -lgdk-x11-2.0 -lgdk_pixbuf-2.0 -lm -lpangocairo-1.0 -lgio-2.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -lglib-2.0 /home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/config/nsinstall -R -m 755 updater ../../../../../dist/bin /home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/config/nsinstall -R -m 644 updater.png ../../../../../dist/bin/icons /home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/config/nsinstall: cannot access updater.png: No such file or directory make[3]: *** [libs] Error 1 make[3]: Leaving directory `/home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/toolkit/mozapps/update/src/updater' make[2]: *** [libs] Error 2 make[2]: Leaving directory `/home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/toolkit/mozapps/update/src' make[1]: *** [libs] Error 2 make[1]: Leaving directory `/home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-optimize/toolkit/mozapps/update' make: *** [default] Error 2 When I revert to: ifeq ($(MOZ_WIDGET_TOOLKIT),gtk2) libs:: updater.png $(INSTALL) $(IFLAGS1) $^ $(DIST)/bin/icons endif it builds fine
Comment 64•15 years ago
|
||
Ah right, it's VPATH confusion at that point. Nevermind, not worth the hassle.
Assignee | ||
Comment 65•15 years ago
|
||
It would be nice to get the linux patch attachment 361880 [details] [diff] [review] checked in. Mac and Windows patches coming soon too. (Mac is attached, but not reviewed yet)
Keywords: checkin-needed
Comment 66•15 years ago
|
||
Comment on attachment 361880 [details] [diff] [review] I misunderstood ted's directions, build was failing. see comment 51 [Checkin: Comment 66] http://hg.mozilla.org/mozilla-central/rev/9911d8835e1b
Attachment #361880 -
Attachment description: I misunderstood ted's directions, build was failing. see comment 51 → I misunderstood ted's directions, build was failing. see comment 51
[Checkin: Comment 66]
Updated•15 years ago
|
Whiteboard: [polish-easy] [polish-visual] [icon-3.1][waiting ddahl] → [needs 1.9.1 landing] [polish-easy] [polish-visual] [icon-3.1][waiting ddahl]
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Assignee | ||
Comment 67•15 years ago
|
||
Needs building/test on windows
Comment 68•15 years ago
|
||
Comment on attachment 363925 [details] [diff] [review] windows ico file (checked in) Built/tested on Windows
Attachment #363925 -
Flags: review+
Assignee | ||
Comment 69•15 years ago
|
||
Comment on attachment 363925 [details] [diff] [review] windows ico file (checked in) this patch needs checkin
Assignee | ||
Comment 70•15 years ago
|
||
Comment on attachment 362751 [details] [diff] [review] Mac Icons Patch (checked in) needs checkin - tested in finder, but not with a mock update.mar
Assignee | ||
Updated•15 years ago
|
Whiteboard: [needs 1.9.1 landing] [polish-easy] [polish-visual] [icon-3.1][waiting ddahl] → [needs 1.9.1 landing] [polish-easy] [polish-visual] [icon-3.1]
Updated•15 years ago
|
Attachment #362751 -
Flags: review+
Updated•15 years ago
|
Attachment #362751 -
Attachment description: Mac Icons Patch → Mac Icons Patch (checked in)
Comment 71•15 years ago
|
||
Comment on attachment 363925 [details] [diff] [review] windows ico file (checked in) Pushed the Windows and Mac icons to mozilla-central http://hg.mozilla.org/mozilla-central/rev/d9edd8cb95cf
Attachment #363925 -
Attachment description: windows ico file → windows ico file (checked in)
Updated•15 years ago
|
Comment 72•15 years ago
|
||
I can verify that the Mac icon shows in the Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2a1pre) Gecko/20090304 Minefield/3.2a1pre build.
Comment 73•15 years ago
|
||
I can verify that the Windows icon shows using the latest nightly running on XP. When I updated the trunk on Windows 7, the icon that showed in the taskbar was the old updater icon, not the new one.
Comment 74•15 years ago
|
||
Marcia, can you try again with a trunk build and Windows 7... I highly suspect it may have been running with a Minefield version from prior to this landing.
Comment 75•15 years ago
|
||
rstrong: This machine did not have a trunk build on it, so I downloaded yesterday's trunk and updated to today's. I can certainly try those STR again but I did not see the updated icon. (In reply to comment #74) > Marcia, can you try again with a trunk build and Windows 7... I highly suspect > it may have been running with a Minefield version from prior to this landing.
Comment 76•15 years ago
|
||
I replayed my STR from Comment 75 on Windows 7 - going from the 20090301 nightly to today's nightly showed the updated icon ( Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090304 Minefield/3.2a1pre). Will check Linux next.
Comment 77•15 years ago
|
||
--> P1, as this bug will require the wider feedback of a beta release or is of sufficient complexity that we should be looking at it sooner, not later.
Priority: P2 → P1
Comment 79•15 years ago
|
||
So, did this ever get checked in on branch, or did we completely miss it (and then miss comment 77)? If it hasn't been checked in, someone should state exactly what needs to be checked into branch, because it's not clear.
Reporter | ||
Comment 80•15 years ago
|
||
This still needs to land on 1.9.1
Assignee | ||
Comment 81•15 years ago
|
||
checkin-needed 1.9.1 - applied to trunk already
Keywords: checkin-needed
Reporter | ||
Updated•15 years ago
|
Attachment #361880 -
Flags: approval1.9.1?
Reporter | ||
Updated•15 years ago
|
Attachment #362751 -
Flags: approval1.9.1?
Reporter | ||
Updated•15 years ago
|
Attachment #363925 -
Flags: approval1.9.1?
Comment 82•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/120b0a019d09
Keywords: checkin-needed → fixed1.9.1
Whiteboard: [needs 1.9.1 landing] [polish-easy] [polish-visual] [icon-3.1] → [polish-easy] [polish-visual] [icon-3.1]
Updated•15 years ago
|
Attachment #361880 -
Flags: approval1.9.1?
Updated•15 years ago
|
Attachment #362751 -
Flags: approval1.9.1?
Updated•15 years ago
|
Attachment #363925 -
Flags: approval1.9.1?
Comment 83•15 years ago
|
||
Verified fix on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090408 Shiretoko/3.5b4pre and Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b4pre) Gecko/20090408 Shiretoko/3.5b4pre and trunk too.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Reporter | ||
Updated•15 years ago
|
Whiteboard: [polish-easy] [polish-visual] [icon-3.1] → [polish-easy] [polish-visual] [icon-3.1][icon-refresh]
Reporter | ||
Comment 84•15 years ago
|
||
This bug's priority relative to the set of other polish bugs is: P2 - Polish issue that is in a secondary interface, occasionally encountered, and is easily identifiable.
Whiteboard: [polish-easy] [polish-visual] [icon-3.1][icon-refresh] → [polish-easy] [polish-visual] [icon-shiretoko][icon-refresh][polish-p2]
You need to log in
before you can comment on or make changes to this bug.
Description
•