Opened 7 years ago

Closed 7 years ago

Last modified 7 years 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:
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 7 years ago.
patch (incl. build failure)
0002-Package-daemon-notifications.patch (7.6 KB ) - added by humdinger 7 years ago.
new patch that actually builds

Download all attachments as: .zip

Change History (16)

comment:1 by humdinger, 7 years ago

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?

by humdinger, 7 years ago

patch (incl. build failure)

comment:2 by humdinger, 7 years ago

patch: 01

comment:3 by pulkomandy, 7 years ago

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 7 years ago by pulkomandy (previous) (diff)

by humdinger, 7 years ago

new patch that actually builds

comment:4 by humdinger, 7 years ago

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 7 years ago by humdinger (previous) (diff)

in reply to:  4 comment:5 by diver, 7 years ago

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 by pulkomandy, 7 years ago

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

comment:7 by humdinger, 7 years ago

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

comment:8 by perelandra, 7 years ago

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 by humdinger, 7 years ago

Resolution: fixed
Status: newclosed

Fixed with hrev51327.

comment:10 by tqh, 7 years ago

Clean builds don't work at all anymore:

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

comment:11 by humdinger, 7 years ago

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

comment:12 by tqh, 7 years ago

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 by perelandra, 7 years ago

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

comment:14 by humdinger, 7 years ago

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.