Closed Bug 954564 Opened 6 years ago Closed 5 years ago

Newly selected contacts don't get scrolled into view.

Categories

(Instantbird :: Contacts window, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: nhnt11)

Details

Attachments

(1 file, 2 obsolete files)

*** 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).
*** 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)
*** 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.
Severity: trivial → minor
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: using cursors for moving in contacts up and down → Newly selected contacts don't get scrolled into view.
Attached patch Patch (obsolete) — Splinter Review
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.
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Attachment #8532880 - Flags: review?(aleth)
Severity: minor → normal
OS: Windows Vista → All
Hardware: x86 → All
Version: 1.1 → trunk
Attached patch Patch v1.1 (obsolete) — Splinter Review
Missed a semicolon.
Attachment #8532880 - Attachment is obsolete: true
Attachment #8532880 - Flags: review?(aleth)
Attachment #8532885 - Flags: review?(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 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?
Attachment #8532885 - Flags: review?(aleth) → review+
Attached patch Patch v1.2Splinter Review
Fixed indentation.
Attachment #8532885 - Attachment is obsolete: true
Attachment #8532900 - Flags: review+
Thanks for looking at this! This has bothered me many times too!
https://hg.mozilla.org/comm-central/rev/da3cf83bf5dd
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.