954225 - Wrong conversation header size after moving the splitter and resizing the window

Status: RESOLVED FIXED

(Instantbird Graveyard :: Conversation, defect)

Description

[Benedikt Pfeifer]

*** Original post on bio 791 at 2011-05-20 12:38:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***

Comment 1

[Benedikt Pfeifer]

Attachment: Screenshot

*** Original post on bio 791 as attmnt 631 at 2011-05-20 12:38:00 UTC ***

After resizing the window a few times to try out the collapsing and expanding bars (conv header/status bar), the conversation header didn't expand all the way. See attached screenshot.

Comment 2

[Benedikt Pfeifer]

Attachment: Screenshot 2

*** Original post on bio 791 as attmnt 632 at 2011-05-20 12:45:00 UTC ***

Screenshot 2 was produced by moving the splitter above the input field up and down between resizes.

Comment 3

[Florian Quèze [:florian]]

*** Original post on bio 791 at 2011-05-20 15:01:04 UTC ***

I can reproduce the problem described in comment 1 on my Mac debug build.

Comment 4

[Florian Quèze [:florian]]

*** Original post on bio 791 at 2011-05-25 16:46:45 UTC ***

Investigating this a bit with DOM inspector, it turns out that moving the splitter sets immediately a "height" attribute on the element with the anonid "conv-top-info". The value of that attribute is the current height of the element at the time the splitter is moved. The presence of this attribute is the cause of the visible effect in attachments 631 and 632.

I haven't understood yet which code sets this height attribute.

Comment 5

[Florian Quèze [:florian]]

Attachment: Workaround

*** Original post on bio 791 as attmnt 655 at 2011-05-25 16:56:00 UTC ***

This is a very ugly workaround, but if we can't understand what happens, I guess we will have to take this.

Comment 6

[Florian Quèze [:florian]]

*** Original post on bio 791 at 2011-05-26 16:26:37 UTC ***

(In reply to comment #4)
> Created an attachment (id=655) [details]
> Workaround
> 
> This is a very ugly workaround, but if we can't understand what happens, I
> guess we will have to take this.

It turns out this seems to be the correct fix. The ugliness was mostly that I didn't understand what happened/why that patch was needed.
I read a bit of nsSplitterFrame.cpp yesterday.

20:42:35 <flo> it takes the child frames (rather than DOM nodes) of the resized area, and for each, it sets the current (=after resize) frame height as the height attribute of the associated content (DOM) node
20:42:48 <flo> that's not what I expected, but it makes sense
20:43:17 <clokep_work> I see.
20:43:48 <flo> so yeah, removing the height attribute where it's clearly undesirable seems like an acceptable solution

Comment 7

[Florian Quèze [:florian]]

*** Original post on bio 791 at 2011-05-26 17:42:42 UTC ***

Pushed attachment 8352398 [details] [diff] [review] (bio-attmnt 655) with a comment explaining why this is needed: https://hg.instantbird.org/instantbird/rev/ffab3491ce07

-> FIXED!