Opened 22 months ago

Closed 21 months ago

Last modified 21 months ago

#13590 closed enhancement (fixed)

[package_daemon] missing an icon in notification window

Reported by: diver Owned by: humdinger
Priority: normal Milestone: Unscheduled
Component: Servers/package_daemon Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

A few issues with package_daemon's notifications:

  • "Package Daemon" - sentence casing
  • package_daemon icon is missing in notification window
  • group name is missing
  • errors are not localized

Attachments (2)

0001-Package-daemon-notifications.patch (3.5 KB) - added by humdinger 22 months ago.
patch (incl. build failure)
0002-Package-daemon-notifications.patch (7.6 KB) - added by humdinger 22 months ago.
new patch that actually builds

Download all attachments as: .zip

Change History (16)

comment:1 Changed 22 months ago by humdinger

I attach a patch that deals with the first three items of this ticket.
Being not much of a coder, I may not understand things correctly... but I never see the function Warn() called that does the notifications. Only the WARN() defined in DebugSupport.h#157.

Are the strings called with WARN() supposed to be translated? There are some difficult ones (for me...), e.g. at Volume.cpp#1209.

Also, more importantly, the patch doesn't build and I don't know why... I pretty much did the same as Brian did for his SoftwareUpdater to get the icon. :)

It fails with:

C++ ../../../generated/objects/haiku/x86_gcc2/release/servers/package/PackageManager.o 
C++ ../../../generated/objects/haiku/x86_gcc2/release/servers/package/Root.o 
In file included from /Source-Haiku/haiku/src/servers/package/PackageManager.h:17,
                 from /Source-Haiku/haiku/src/servers/package/Root.cpp:28:
/Source-Haiku/haiku/headers/os/interface/Bitmap.h:117: use of `BPrivate' is ambiguous
/Source-Haiku/haiku/headers/os/support/Archivable.h:17:   first declared as `BPrivate' here
/Source-Haiku/haiku/headers/private/package/ActivationTransaction.h:18:   also declared as `BPackageKit::BPrivate' here
/Source-Haiku/haiku/headers/os/interface/Bitmap.h:117: parse error before `;'

Seems to complain about that little BBitmap... Any idea how to solve that?

Changed 22 months ago by humdinger

patch (incl. build failure)

comment:2 Changed 22 months ago by humdinger

Has a Patch: set

comment:3 Changed 22 months ago by pulkomandy

You can use ::BPrivate instead of BPrivate in BBitmap.h to avoid this problem. It tells the compiler to use the BPrivate from the top level, and not the one from the package kit.

The Warn function is called directly from the package kit, in src/kits/package/manager/PackageManager.cpp (fUserInteractionHandler->Warn). These should be translated.

WARN() gets to syslog, and should not be translated because we want devs to be able to read syslogs whne people report a bug.

Also, this _GetIcon seems a bit hacky. Shouldn't this be the default behavior of BNotification? Why do apps need to do anything special to get their own icon shown?

Last edited 22 months ago by pulkomandy (previous) (diff)

Changed 22 months ago by humdinger

new patch that actually builds

comment:4 Changed 22 months ago by humdinger

Thanks PulkoMandy. I attached a new patch with the changes to Bitmap.h that make it actually compile. Much obliged, if you could have a quick look (and apply the patch, if you want).

No idea about the icon. It sure would be nicer if the notification defaults to the calling app's icon if none is set. If possible...

Edited to add: please have a look at the Jamfile additions to the libpackage.so kit. I guess it's alright, but am not sure...

Last edited 22 months ago by humdinger (previous) (diff)

comment:5 in reply to:  4 Changed 22 months ago by diver

Replying to humdinger:

No idea about the icon. It sure would be nicer if the notification defaults to the calling app's icon if none is set. If possible...

I second that! This way we'll reduce the number of cases with missing icons. IIRC Vision suffers from it.

comment:6 Changed 21 months ago by pulkomandy

Moved the _GetIcon code to BNotification in hrev51299. You can now remove it from the patch and also from SoftwareUpdater.

comment:7 Changed 21 months ago by humdinger

What about the friend class ::BPrivate::BPrivateScreen; in Bitmap.h ? Should that stay in the patch or not?

comment:8 Changed 21 months ago by perelandra

SoftwareUpdater has been changed in hrev51325 so it doesn't add the icon again, allows the one created by BNotification to be used. Verified it looks the same as the one I was adding.

comment:9 Changed 21 months ago by humdinger

Resolution: fixed
Status: newclosed

Fixed with hrev51327.

comment:10 Changed 21 months ago by tqh

Clean builds don't work at all anymore:

don't know how to make <src!kits!package!solver>PackageManager.cpp

comment:11 Changed 21 months ago by humdinger

I'm very sure it's my fault, but I don't know why... Help appreciated.

comment:12 Changed 21 months ago by tqh

It's probably hrev51327 's Jamfile changes. Not sure what DoCatalogs do, and why it takes a source file. Seems weird to me. Building with this commit reverted. Hope it completes.

comment:13 Changed 21 months ago by perelandra

You are adding PackageManager.cpp to both package_daemon and libpackage.so ?

comment:14 Changed 21 months ago by humdinger

Thanks to korli, it's fixed with hrev51338. Apparently

DoCatalogs rule needs to find sources. SubInclude should stay at the end.

@perelandra: there's a PackageManager.cpp source file in both kit and server.

Note: See TracTickets for help on using tickets.