288949 - List layout broken after #288117 resolving

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Marian POPESCU]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050404 Firefox/1.0+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050404 Firefox/1.0+

After fixing bug #288117, there are some problems with the list layout.
The whole area intended to display the list is misplaced and there are some
painting artifacts.
The attached screenshots explain better than the word what is the matter.
I attached also a test case (HTML file and CSS file).

Reproducible: Always

Steps to Reproduce:
1.Download the testcase files on a folder
2.Open with Firefox - Gecko/20050404 Firefox/1.0+ - Linux ocean Clobber release
finished on 04/04 01:55
3.Look at the list layout

Actual Results:  
The two vertical tabs are not on the correct position; they are misplaced to the
top.

Expected Results:  
The two vertical tabs should be diplayed aligned to the right container.

Comment 1

[Marian POPESCU]

Attachment: Incorrect list layout

Comment 2

[Marian POPESCU]

Attachment: Correct list layout

Comment 3

[Marian POPESCU]

Attachment: Testcase - zipped

Please save this testcase on a folder and uncompress it before use

Comment 4

[Martijn Wargers (dead)]

Well, I'm not seeing this bug. The testcase looks still correct to me with the
20050404 nightly trunk build on WinXP.

Comment 5

[Marian POPESCU]

(In reply to comment #4)
> Well, I'm not seeing this bug. The testcase looks still correct to me with the
> 20050404 nightly trunk build on WinXP.
> 
I don't know about Windows builds, but I am using a Linux one ...
That's why I put OS=Linux.

Comment 6

[Marian POPESCU]

(In reply to comment #5)
> (In reply to comment #4)
> > Well, I'm not seeing this bug. The testcase looks still correct to me with the
> > 20050404 nightly trunk build on WinXP.
> > 
> I don't know about Windows builds, but I am using a Linux one ...
> That's why I put OS=Linux.
To be more specific, I use:
- Fedora Core 3; 
- kernel 2.6.10-1.770_FC3;
- xorg-x11-6.8.2-1.FC3.13;
- kde 3.4.0.

Comment 7

[Andrew Schultz]

Attachment: testcase

testcase exhibits the described bug with linux trunk 2005040402
the inner (red) div appears in the top left corner of the browser AND where
it's supposed to be (20px from the top and left edge)

Comment 8

[Andrew Schultz]

confirmed with linux trunk 2005040402

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is fixed in my upcoming patch for bug 240276. The problem is that widget
reparenting isn't implemented in GTK2 so when
nsCSSFrameConstructor::FinishBuildingScrollFrame tries to reparent the scrolled
content to the inner anonymous view, it doesn't fully happen.

Comment 10

[Marian POPESCU]

I guess I forgot to say that the regression occured sometime after 2005/03/31.
Before that, the content on my testcase was rendered correctly.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Fix this regression by avoiding reparenting views in FinishBuildingScrollFrame;
this requires widget reparenting to work, which it doesn't on GTK2. Add an
nsIFrame API so that the correct parent view can be found when it's located.

This patch also fixes another regression I found. Currently listboxes are
getting widgets because NeedsClipWidget is starting the search for
nsIFormControlFrame at the scrollframe's parent, but for listboxes it's the
scrollframe itself that implements nsIFormControlFrame.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 179692 [details] [diff] [review]
fix

Oops, that was completely the wrong patch. I'll attach the patch I meant to
submit.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: real fix

There you go ... much less daunting :-)

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 179979 [details] [diff] [review]
real fix

This fixes visual regressions and probably some crashers too. Pretty low risk
(much less risky than the code it replaces :-) )

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 179979 [details] [diff] [review]
real fix

a=asa

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in