Created attachment 169976 [details] [review]
[MessageTray] fix allocation in the title-too-long case

Previously we were hiding the banner label if the title was too long,
but this causes queue_relayout() warnings. Instead, just allocate it
a width of 0 in that case, which works fine.

Created attachment 169981 [details] [review]
[MessageTray] fix allocation in the title-too-long case

Oops, if there's a body as well, it still shows as "...". So just
tweak opacity instead...

Review of attachment 169981 [details] [review]:

This makes the banner flicker when _bannerBoxAllocate() gets called in the expanded mode. Which seems to happen sometimes. For example, when you quickly move your mouse out and back into the notification.

It seems to work fine if you just remove hide() and show() calls. The only question is whether it is ok not to call allocate() on an actor that is not hidden. If not, can we call allocate for this._bannerLabel with a 0 width box? Did that result in "..." still showing up?

Created attachment 170249 [details] [review]
[MessageTray] fix allocation in the title-too-long case

just removing the hide and show won't work. this fixes it by only resetting
opacity to 255 when updating the notification, rather than doing it on each
call to _bannerBoxAllocate()

Review of attachment 170249 [details] [review]:

::: js/ui/messageTray.js
@@ +217,3 @@
         banner = banner ? _cleanMarkup(banner.replace(/\n/g, '  ')) : '';
         this._bannerLabel.clutter_text.set_markup(banner);
+        this._bannerLabel.opacity = 255;

This makes the banner flicker when an expanded notification with the banner in the body gets updated (as in with libnotify's test-replace). It can be fixed by checking this.expanded here. However, we already have a check for the case when a notification that is expanded got a shorter banner that should be shown in the top line, in which case we also set this._bannerLabel.opacity to 255. We can combine these two checks after we figure out bannerFits in the case we are displaying some of the banner in _bannerBoxAllocate() .

I think this should work there.

if (!this.expanded || (bannerFits && !this._contentArea && !this._actionArea))
    this._bannerLabel.opacity = 255;

You can then move the corresponding comment and remove the corresponding code in the end of _bannerBoxAllocate() .

Created attachment 170440 [details] [review]
[MessageTray] fix allocation in the title-too-long case

OK, I ended up doing it a little bit differently, but I think this is
equivalent

Review of attachment 170440 [details] [review]:

Looks good!

Attachment 170440 [details] pushed as 828ad34 - [MessageTray] fix allocation in the title-too-long case