Closed
Bug 380842
Opened 17 years ago
Closed 17 years ago
[regression] table-cell :before/:after pseudo-element set to display:block show as display:inline
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
VERIFIED
FIXED
mozilla1.9alpha5
People
(Reporter: phiw2, Assigned: bzbarsky)
References
Details
(4 keywords)
Attachments
(3 files, 1 obsolete file)
598 bytes,
text/html
|
Details | |
4.04 KB,
patch
|
Details | Diff | Splinter Review | |
1.41 KB,
patch
|
dveditz
:
approval1.8.1.5+
dveditz
:
approval1.8.0.13+
|
Details | Diff | Splinter Review |
td:before or td:after {display:block} shows as inline box instead of block box In the test case the text '[before]' and '[after]' should show on their own line. works correctly for paragraphs etc. Fails for table-cell (td). On trunk this regressed between 20070415-04 (OK) and 20070416-04 (fails) Branch 1.8.1.4 pre also fails the test. Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.1.4pre) Gecko/20070515 BonEcho/2.0.0.4pre ID:2007051505 Firefox 2.0.0.3 works correctly http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-04-15+04%3A00%3A00&maxdate=2007-04-16+04%3A00%3A00&cvsroot=%2Fcvsroot suspicion: bug 323656, as this landed on branch as well.
![]() |
Reporter | |
Updated•17 years ago
|
Summary: [regression] table-cell before/after set to display:block show as inline → [regression] table-cell :before/:after pseudo-element set to display:block show as display:inline
![]() |
Assignee | |
Comment 1•17 years ago
|
||
Yeah, this is indeed a regression from bug 323656 and a problem on both branches... I realize it's really late in the cycle, and this correctness regression might not be quite enough to stop ship. But if something else comes up to make us respin, we should take this too. And we should definitely take this next cycle.
Blocks: 323656
Component: Layout → Style System (CSS)
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.12?
OS: Mac OS X → All
QA Contact: layout → style-system
Hardware: Macintosh → All
![]() |
Assignee | |
Comment 2•17 years ago
|
||
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #265007 -
Flags: superreview?(dbaron)
Attachment #265007 -
Flags: review?(dbaron)
Comment 3•17 years ago
|
||
Comment on attachment 265007 [details] [diff] [review] Trunk patch > // XXX IsBlockInside? (except for the marker bit) > if (parentDisplay->IsBlockLevel() || >- parentDisplay->mDisplay == NS_STYLE_DISPLAY_INLINE_BLOCK) { >+ parentDisplay->mDisplay == NS_STYLE_DISPLAY_INLINE_BLOCK || >+ parentDisplay->mDisplay == NS_STYLE_DISPLAY_TABLE_CELL) { I think you want to add TABLE_CAPTION in addition to TABLE_CELL, no? (I'd suggest using IsBlockInside like the comment says, but really this code should be gradually moving towards if(PR_TRUE), since the restrictions aren't in CSS2.1 anymore. So we may as well keep TABLE in there now that we've supported it.) >Index: layout/reftests/bugs/380842-1.html Could you also test at least one of the cases where display:block before and after pseudos *are* supposed to change to inlines? r+sr=dbaron with that
Attachment #265007 -
Flags: superreview?(dbaron)
Attachment #265007 -
Flags: superreview+
Attachment #265007 -
Flags: review?(dbaron)
Attachment #265007 -
Flags: review+
![]() |
Assignee | |
Comment 4•17 years ago
|
||
Attachment #265007 -
Attachment is obsolete: true
![]() |
Assignee | |
Comment 5•17 years ago
|
||
Attachment #265010 -
Flags: approval1.8.1.5?
Attachment #265010 -
Flags: approval1.8.1.4?
Attachment #265010 -
Flags: approval1.8.0.13?
Attachment #265010 -
Flags: approval1.8.0.12?
![]() |
Assignee | |
Comment 6•17 years ago
|
||
Checked in on trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha5
Comment 7•17 years ago
|
||
(In reply to comment #3) > >Index: layout/reftests/bugs/380842-1.html > > Could you also test at least one of the cases where display:block before and > after pseudos *are* supposed to change to inlines? Actually, since the spec doesn't support doing this anymore, maybe we shouldn't test it (since it's actually incorrect). Sorry to keep changing my mind here...
![]() |
Assignee | |
Comment 8•17 years ago
|
||
I think it's worth testing until we actually decide to change our impl, because changing the rule node code without changing the frame constructor to do {ib} splits in this situation will probably lead to problems that we want to catch, and this test might do that, I think.
Comment 9•17 years ago
|
||
I'd just like to note that this regression broke ChatZilla's faces motifs (http://chatzilla.hacksrus.com/faces.pl). I realize that we're an obscure usecase and all, but yeah... It would be wonderful if bz's patch was shipped on branch as well.
Updated•17 years ago
|
Flags: in-testsuite?
Comment 10•17 years ago
|
||
That's what I get for flipping the flag before skimming tinderbox checkins: http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/reftests/bugs&command=DIFF_FRAMESET&file=reftest.list&rev1=1.66&rev2=1.67&root=/cvsroot
Flags: in-testsuite? → in-testsuite+
Updated•17 years ago
|
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?
Updated•17 years ago
|
Attachment #265010 -
Flags: approval1.8.1.4?
Attachment #265010 -
Flags: approval1.8.0.12?
Comment 11•17 years ago
|
||
Comment on attachment 265010 [details] [diff] [review] Merged to branch version Approved for 1.8.1.5, a=dveditz for release-drivers
Attachment #265010 -
Flags: approval1.8.1.5? → approval1.8.1.5+
Updated•17 years ago
|
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.5+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.13+
![]() |
Assignee | |
Comment 12•17 years ago
|
||
I think we should also fix this on the 1.8.0 branch; this is a pretty serious layout regression, and just leaving branch like this is very suboptimal... Who'd be doing approvals for that branch? Would it make sense to approve bugs applicable to both branches for both? Holding off on checking this in pending answers, because landing on both branches at once is a lot less time-consuming than doing it separately...
Comment 13•17 years ago
|
||
Comment on attachment 265010 [details] [diff] [review] Merged to branch version approved for 1.8.0.13, a=dveditz
Attachment #265010 -
Flags: approval1.8.0.13? → approval1.8.0.13+
![]() |
Assignee | |
Comment 14•17 years ago
|
||
Checked in on both branches. Thanks for the approvals!
Keywords: fixed1.8.0.13,
fixed1.8.1.5
Comment 15•17 years ago
|
||
Verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.5pre) Gecko/20070709 BonEcho/2.0.0.5pre and the reftest. FWIW, I filed bug 387600 for the discrepancy between the reference renderings I see between trunk and branch.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.5 → verified1.8.1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•