After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 628241 - Messagetray shows scrolling area regardless whether it is needed or not
Messagetray shows scrolling area regardless whether it is needed or not
Status: RESOLVED DUPLICATE of bug 623970
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Dan Winship
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-08-29 10:01 UTC by drago01
Modified: 2010-08-30 21:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Screenshot showing the issue (10.80 KB, image/png)
2010-08-29 10:01 UTC, drago01
  Details
[Messagetray] Don't create the label in addBody when no text is passed (1.27 KB, patch)
2010-08-29 10:33 UTC, drago01
rejected Details | Review
[MessageTray] Avoid calling addBannerBody whit an empty text in _bannerBoxAllocate (1.40 KB, patch)
2010-08-30 21:16 UTC, drago01
rejected Details | Review

Description drago01 2010-08-29 10:01:34 UTC
Created attachment 168992 [details]
Screenshot showing the issue

After commit 93d3270566a2b4873ec72417d68096909e0ad4a8 (https://bugzilla.gnome.org/show_bug.cgi?id=627985) we always show the scrolling area regardless whether it is needed or not (see screenshot).
Comment 1 drago01 2010-08-29 10:33:46 UTC
Created attachment 168993 [details] [review]
[Messagetray] Don't create the label in addBody when no text is passed

In case we don't have any text yet this could result into a wrong allocation,
and causing us to show a scrolling area even for only one line texts.
Comment 2 Dan Winship 2010-08-30 15:43:03 UTC
how does trying to add an empty label cause this bug?
Comment 3 Owen Taylor 2010-08-30 19:10:23 UTC
(decided against the NEEDINFO marking. Will leave NEEDINFO to mean "need info from the reporter")
Comment 4 drago01 2010-08-30 20:52:04 UTC
(In reply to comment #2)
> how does trying to add an empty label cause this bug?

Because we add "dead" labels that fill up space, as each call to addBody adds a new actor.
Comment 5 drago01 2010-08-30 21:15:31 UTC
(In reply to comment #4)
> (In reply to comment #2)
> > how does trying to add an empty label cause this bug?
> 
> Because we add "dead" labels that fill up space, as each call to addBody adds a
> new actor.

As for the question why we do that:

Commit 93d3270566a2b4873ec72417d68096909e0ad4a8 adds a call to idle handle which calls _addBannerBody() in _bannerBoxAllocate() ... so when _bannerBoxAllocate() is called multple times it queues  a call to to _addBannerBody(), the first one sets _bannerBodyText to null and therefore the later ones add null as text.
Comment 6 drago01 2010-08-30 21:16:05 UTC
Created attachment 169129 [details] [review]
[MessageTray] Avoid calling addBannerBody whit an empty text in _bannerBoxAllocate

Commit 93d3270566a2b4873ec72417d68096909e0ad4a8 adds a call to idle handle which calls _addBannerBody() in _bannerBoxAllocate() ... so when _bannerBoxAllocate() is called multple times it queues  a call to to _addBannerBody(), the first one sets _bannerBodyText to null and therefore the later ones add null as text.

Fix that by checking for null in the idle handler.

------

Different fix
Comment 7 Marina Zhurakhinskaya 2010-08-30 21:40:09 UTC
The problem you are solving here is solved in this patch:
https://bugzilla.gnome.org/show_bug.cgi?id=623970#c11

We check if this._bannerBodyText is no null inside _addBannerBody() in it.

*** This bug has been marked as a duplicate of bug 623970 ***
Comment 8 Marina Zhurakhinskaya 2010-08-30 21:43:42 UTC
Review of attachment 169129 [details] [review]:

Thanks! This patch looks right, but this bug is fixed with a different patch that reorganizes the code for adding the banner body a bit more.
Comment 9 drago01 2010-08-30 21:46:55 UTC
Review of attachment 168993 [details] [review]:

OK, so lets get this one of the patch queue too.