acid2 is broken (2006122706)

VERIFIED FIXED

Status

()

Core
Layout: Tables
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: Peter6, Assigned: Bernd)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

11 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a2pre) Gecko/20061227 Minefield/3.0a2pre ID:2006122706 [cairo]

see screenshot

works in 20061227_0057_0121_firefox-3.0a2pre.en-US.win32
fails in 20061227_0619_0645_firefox-3.0a2pre.en-US.win32

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1167209820&maxdate=1167229139

->Bug 243159

Comment 1

11 years ago
Created attachment 249817 [details]
another screenshot

This is what I get (and also got with 2006122604, so that could be another bug). When opened in a background tab, Acid2 passes. That is, I can't reproduce attachment 249815 [details].

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20061227 Minefield/3.0a2pre ID:2006122704 [cairo]

Updated

11 years ago
Summary: acid 2 is broken (20061227) → acid2 is broken (20061227)

Comment 2

11 years ago
(In reply to comment #1)
> That is, I can't reproduce attachment 249815 [details].
> 
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20061227
> Minefield/3.0a2pre ID:2006122704 [cairo]

Oops. Of course the nightly is not affected, because the patch for bug 243159 was checked in later that day.
Summary: acid2 is broken (20061227) → acid2 is broken (2006122706)

Updated

11 years ago
Attachment #249817 - Attachment is obsolete: true
(Assignee)

Comment 3

11 years ago
yes thats row 14 -> table pseudos
Assignee: nobody → bernd_mozilla
(In reply to comment #3)
> yes thats row 14 -> table pseudos
> 

Odd. Because it is row 13 (as defined on http://www.webstandards.org/action/acid2/guide/ ) that does not display correctly for me.
Flags: blocking1.9?
(Assignee)

Comment 5

11 years ago
Created attachment 249850 [details] [diff] [review]
patch
(Assignee)

Comment 6

11 years ago
the key is to remove the table special handling at https://bugzilla.mozilla.org/attachment.cgi?id=249850&action=diff#mozilla/layout/base/nsCSSFrameConstructor.cpp_sec6
the rest are consequence of it.
(In reply to comment #6)
> the key is to remove the table special handling at
> https://bugzilla.mozilla.org/attachment.cgi?id=249850&action=diff#mozilla/layout/base/nsCSSFrameConstructor.cpp_sec6
> the rest are consequence of it.
> 
I can verify that this patch resolves the issue.  I especially like that the fix is to remove special case code rather than to add more.
(Assignee)

Comment 8

11 years ago
Comment on attachment 249850 [details] [diff] [review]
patch

Boris I am not confident about the early return in ConstructFrameByDisplayType
Attachment #249850 - Flags: superreview?(bzbarsky)
Attachment #249850 - Flags: review?(bzbarsky)
Bad news!  Although acid 2 renders correctly with my build including this patch, my.yahoo.com does not. :-)
Created attachment 249899 [details]
test that fails with above patch installed

This is a reduced testcase of the issue with the my.yahoo.com page if the above patch is applied.
(Assignee)

Comment 11

11 years ago
Comment on attachment 249850 [details] [diff] [review]
patch

-    rv = aState.AddChild(aNewOuterFrame, *frameItems, disp, aContent,
-                         outerStyleContext, parentFrame, aAllowOutOfFlow,
-                         aAllowOutOfFlow);
cannot be replaced by a single 

 if (NS_SUCCEEDED(rv) && !aHasPseudoParent) {
+        aFrameItems.AddChild(newFrame);
+      }
Attachment #249850 - Flags: superreview?(bzbarsky)
Attachment #249850 - Flags: review?(bzbarsky)
(Assignee)

Comment 12

11 years ago
fixed by backout of bug 243159
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

11 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a2pre) Gecko/20061229 Minefield/3.0a2pre ID:2006122905 [cairo]

VERIFIED
Status: RESOLVED → VERIFIED
Wait.  So why not use that last patch but use the right AddChild method?  Or is that the plan but you just plan to do more testing in bug 243159 first?
Also, need to add these testcases to a regression test suite of some sort.
Flags: blocking1.9? → in-testsuite-
(Assignee)

Comment 16

11 years ago
>Or is that the plan but you just plan to do more testing in bug 243159 first?
Boris, thats the plan, comment 10 made it obvious to me that the only description for what is going on here is: "chaltura". A patch of this scope has to pass rtest. I don't want to take the tree as hostage + I don't like to own bugs where every Mozilla fan boy is on CC.
Blocks: 289480
Flags: in-testsuite- → in-testsuite?
You need to log in before you can comment on or make changes to this bug.