954564 - Newly selected contacts don't get scrolled into view.

Status: RESOLVED FIXED

(Instantbird Graveyard :: Contacts window, defect)

Description

[Instantbot]

*** Original post on bio 1131 by Tomáš Komárek <tomaskom.cz AT seznam.cz> at 2011-11-01 21:20:00 UTC ***

In contacts window, Instantbird does not keep the marked contact visible (you can have marked target out of visible part of contacts list when using up or down cursors).

Comment 1

[Instantbot]

*** Original post on bio 1131 by Tomáš Komárek <tomaskom.cz AT seznam.cz> at 2011-11-02 16:40:15 UTC ***

To be clear, here's how to see it on your own:

1. have contacts window opened
2. have more contacts than is capacity of the window (e.g. show offline ones) -> scroll bar appears
3. srcoll to the top
4. click on any contact and by pressing down arrow, change the highlighted contact
5. when you reach the last visible contact and still keep pressing down arrow, you don't see the highlighted one (window doesn't automatically scroll down)

Comment 2

[Patrick Cloke [:clokep]]

*** Original post on bio 1131 at 2011-11-02 16:42:55 UTC ***

Is this a regression from the conversations on hold check-ins? (Specifically the one moving the scroll list outside of the listbox?)

Confirming.

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch

This bug bothered me one too many times, so I fixed it.
Note that this problem also happens when you type the first few letters of a contact's name - the contact gets selected but you have to scroll manually to find it.
This patch fixes both the above issue and the one in the original bug report.

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch v1.1

Missed a semicolon.

Comment 5

[aleth [:aleth]]

Comment on attachment 8532885 [details] [diff] [review]
Patch v1.1

Review of attachment 8532885 [details] [diff] [review]:
-----------------------------------------------------------------

::: im/content/blist.js
@@ +974,5 @@
> +  // Usually, a scrollable richlistbox will ensure that a newly selected item is
> +  // automatically scrolled into view. However, buddylistbox and convlistbox are
> +  // both zero-flex children of a flexible notification box, and don't have
> +  // scrollboxes themselves - so it's necessary to manually set the scroll of the
> +  // notification box when an item is selected to ensure its visibility.

Just checking: does this mean that both element.scrollIntoView(); and scrollbox.ensureElementIsVisible(); don't work here, because the element isn't a direct child node?

Comment 6

[aleth [:aleth]]

Comment on attachment 8532885 [details] [diff] [review]
Patch v1.1

Review of attachment 8532885 [details] [diff] [review]:
-----------------------------------------------------------------

It turns out scrollIntoView always scrolls, even if the element is already visible, and to check whether the element is visible we'd need to keep essentially all of this code anyway. Therefore r+

::: im/content/blist.js
@@ +985,5 @@
> +    // The offset of the top of the notification box from the top of the item.
> +    let offsetAboveTop = notifboxBounds.top - itemBounds.top;
> +    // The offset of the bottom of the item from the bottom of the notification box.
> +    let offsetBelowBottom = itemBounds.top + itemBounds.height -
> +                       (notifboxBounds.top + notifboxBounds.height);

Nit: is the indentation correct here?

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch v1.2

Fixed indentation.

Comment 8

[Patrick Cloke [:clokep]]

Thanks for looking at this! This has bothered me many times too!

Comment 9

[(inactive) Nihanth Subramanya [:nhnt11]]

https://hg.mozilla.org/comm-central/rev/da3cf83bf5dd