Closed Bug 955586 Opened 10 years ago Closed 10 years ago

Bubbles' last message sometimes doesn't auto-scroll

Categories

(Instantbird Graveyard :: Conversation, defect)

x86
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: aleth)

Details

Attachments

(2 files)

*** Original post on bio 2147 at 2013-09-02 22:07:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached patch PatchSplinter Review
*** Original post on bio 2147 as attmnt 2813 at 2013-09-02 22:07:00 UTC ***

I haven't been able to consistently reproduce this but I've been observing it for a while, and my guess is that it happens when the size of the "lastMessage" element switches immediately to a height > 10px, but less than 5 minutes (ie. no text displayed yet). This can happen either when restoring a conversation from hold, or with a non-selected tab.

I haven't fully understood what's exactly happening, but I have a strong suspicion on the line the attached patch replaces. I think that line is quite fragile. I'm wondering if the patch wouldn't cause a second reflow though :-S.
Attachment #8354583 - Flags: feedback?(aleth)
Comment on attachment 8354583 [details] [diff] [review]
Patch

*** Original change on bio 2147 attmnt 2813 at 2013-09-04 14:41:54 UTC ***

The idea of this patch seems plausible, but I have not been able to reproduce anything conclusive. 

I wonder if the "right" thing to do (also on general grounds) would be to expose convbrowser._autoScrollEnabled and if required convbrowser._updateAutoScrollEnabled to the message styles, rather than keep the duplication we have at present.

Another suspicion: are MutationObservers potentially batched over a long time when a tab does not have focus?
Attachment #8354583 - Flags: feedback?(aleth) → feedback+
*** Original post on bio 2147 at 2013-09-04 14:43:16 UTC ***

(PS - note we already expose scrollToElement in http://lxr.instantbird.org/instantbird/source/chat/content/convbrowser.xml#825)
Comment on attachment 8354583 [details] [diff] [review]
Patch

*** Original change on bio 2147 attmnt 2813 at 2013-09-09 22:05:13 UTC ***

I'm confused about what the next step is here, so setting r? to clarify if you think we should try this patch in nightlies, or dig further (I don't have any idea to explore right now).

(In reply to comment #1)

> Another suspicion: are MutationObservers potentially batched over a long time
> when a tab does not have focus?

This isn't the problem. When a tab doesn't have focus, we don't update the last message interval.
Attachment #8354583 - Flags: review?(aleth)
*** Original post on bio 2147 at 2013-09-11 15:44:06 UTC ***

As per comment #1, I would like to understand why this code seems to duplicate http://lxr.instantbird.org/instantbird/source/chat/content/convbrowser.xml#270
and so whether this patch could be done in a cleaner way if instead of recalculating the value we just used convbrowser._autoScrollEnabled.
*** Original post on bio 2147 as attmnt 2899 at 2013-09-23 21:59:00 UTC ***

I've occasionally seen this bug recently, but I don't have STR. This patch implements my suggestion above to see if it helps as well...
Attachment #8354669 - Flags: feedback?(florian)
Comment on attachment 8354669 [details] [diff] [review]
Alternative for testing

*** Original change on bio 2147 attmnt 2899 at 2013-09-24 10:41:19 UTC ***

This seems reasonable. I guess the only way to know if it helps is to check it in and try in nightlies.
Attachment #8354669 - Flags: feedback?(florian) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 2147 at 2013-09-25 00:34:42 UTC ***

http://hg.instantbird.org/instantbird/rev/e6e6695dd686

I'll leave this open, aleth or Florian, please close this if the issue is fixed.
Assignee: nobody → aleth
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
*** Original post on bio 2147 at 2013-09-30 23:46:50 UTC ***

As far as we know this is fixed.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8354583 [details] [diff] [review]
Patch

*** Original change on bio 2147 attmnt 2813 at 2013-09-30 23:48:39 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354583 - Flags: review?(aleth)
You need to log in before you can comment on or make changes to this bug.