Closed Bug 63895 Opened 24 years ago Closed 11 years ago

positioned internal table elements not abs pos containing block

Categories

(Core :: Layout: Positioned, defect)

Product:
Core
Component:
Layout: Positioned
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla31
Tracking Flags:
Tracking Status
firefox30 --- fixed
firefox31 --- verified
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed
fennec + ---

People

(Reporter: stephenc, Assigned: seth)

References

(Depends on 2 open bugs, Blocks 2 open bugs,
URL
)

Details

(Keywords: dev-doc-needed, relnote, testcase, Whiteboard: [awd:tbl])

Attachments

(12 files, 18 obsolete files)

Testcase #1, abs in rel
672 bytes, text/html
Details
Testcase #2, abs in abs
660 bytes, text/html
Details
Testcase
1.43 KB, text/html
Details
Reftest
2.53 KB, patch
roc
: review+
seth
: checkin+
Details | Diff | Splinter Review
Testcase #3, positioning within cell of real table, css table and div
1.94 KB, text/html
Details
Testcase #4 (WORKS OK), positioning in table, css table and div
1.63 KB, text/html
Details
Testcase #5, the yellow div should be inside the red (table-cell) div
605 bytes, text/html
Details
(Part 3) - Add tests for positioned internal table objects serving as absolute containing blocks.
27.51 KB, patch
dbaron
: review+
seth
: checkin+
Details | Diff | Splinter Review
(Part 1) - Add helper methods for retrieving overflow areas in the frame's local coordinate space.
3.62 KB, patch
seth
: checkin+
Details | Diff | Splinter Review
(Part 2) - Support table parts as absolute containing blocks.
37.78 KB, patch
seth
: checkin+
Details | Diff | Splinter Review
(Part 5) - Add more tests for positioned internal table objects serving as absolute containing blocks.
31.02 KB, patch
dbaron
: review+
seth
: checkin+
Details | Diff | Splinter Review
(Part 4) - Propagate overflow of relatively positioned table parts to the table frame.
3.90 KB, patch
lsblakk
: approval-mozilla-aurora+
seth
: checkin+
Details | Diff | Splinter Review
From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.5; Windows 98) BuildID: 2000122805 Mozilla seems to be disregarding a relative positioned table as an ancestral containing block, positioning the absolute positioned div inside the table from the edge of the viewport rather than from the edge of the table. Reproducible: Always Steps to Reproduce: Open supplied URL Actual Results: Overlayed div is positioned from viewport. Expected Results: Overlayed div should be positioned from relative positioned containing table. The page *does* render properly if I encase both the table and div inside another div and set that div to relative (switching the margin and other properties currently on the table to the div, of course). But, both table and div are valid containing blocks according to CSS2. Why does the positioning work inside div but not inside table?
Reporter Several of your <INPUT...> statements are lacking the neccesary / at the end of them as stated in http://www.w3.org/TR/2000/REC-xhtml1-20000126/ My guess is you might want to try and fix those and see if it still occurs.
As predicted, fixing the very minor unrelated errors on the page does nothing for the positioning. The errors are due to an external HTML library and aren't under my control. To satisfy the pedantic people, I put the script output into an HTML file and fixed things there. I've changed the bug's URL to point to this page instead. This page validates as XHTML 1.0 Strict but the bad positioning remains. As I noted in the original text, the exact same page (with or without bad input tags) is positioned properly if the entire page body is wrapped in a div and position based off that. This should not be necessary according to the spec.
URL: http://fs1.theiqgroup.com/cgi-bin/iqc... → http://fs1.theiqgroup.com/iqclogin.html
Ok going ahead and marking it NEW since I see it as well on the Linux 2001010221 Build.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 98 → All
Hardware: PC → All
Netscape's standard compliance QA team reorganised itself once again, so taking remaining non-tables style bugs. Sorry about the spam. I tried to get this done directly at the database level, but apparently that is "not easy because of the shadow db", "plus it screws up the audit trail", so no can do...
QA Contact: chrisd → ian
QA Contact: ian → amar
*** Bug 87559 has been marked as a duplicate of this bug. ***
retitling, ->Layout
Assignee: pierre → karnaze
Component: Style System → Layout
QA Contact: amar → petersen
Summary: Absolute positioned div inside relative positioned table using viewport instead of table as containing block → relative/absolute positioned table not abs pos containing block
Whiteboard: [awd:tbl][NEED TESTCASE]
This code probably needs to be fixed in 2 places, the second of which (for dynamic changes) is nsCSSFrameConstructor::GetAbsoluteContainingBlock.
Temporarily moving to future until a milestone can be assigned.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
URL (http://fs1.theiqgroup.com/iqclogin.html) is 404, removing it.
URL: http://fs1.theiqgroup.com/iqclogin.html
Keywords: testcase
Whiteboard: [awd:tbl][NEED TESTCASE] → [awd:tbl]
Attachment #76045 - Attachment description: Testcase → Testcase #1, abs in rel
*** Bug 142368 has been marked as a duplicate of this bug. ***
Is there any chance this is going to make Moz 1.0?
Identical behavior also occurs if the absolutely positioned DIV is inside a staticly positioned (default) DIV. It ignores the static DIV and positions the absolute DIV relative to the page as a whole.
Never mind. Since the parent DIV isn't positioned, it's not considered a containing block, so my comment doesn't mean anything.
This seems like a pretty significant bug, considering that CSS2 positioning is supposed to end dependence on table-based layouts.
I looked at the source for nsCSSFrameConstructor::GetAbsoluteContaining Block: http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSFrameConstructor.cpp#7615 and I found the following comment: 7624 // Is it positioned? 7625 // If it's a table then ignore it, because for the time being tables 7626 // are not containers for absolutely positioned child frames Does anyone know the reason for this change in behavior? Derek
.
Assignee: karnaze → position
Status: ASSIGNED → NEW
Component: Layout → Layout: R & A Pos
QA Contact: petersen → madhur
Should this bug have css2 keyword added? It would be really nice to have this fixed for 1.3. This bug is a pet peeve of mine, and it's been around for a very long time :)
Priority: -- → P4
*** Bug 186581 has been marked as a duplicate of this bug. ***
I've run into this one too. For the time being, I'm surrounding my tables with a rel. pos. DIV element to make it work. Ugly hack, though.
Arrgh! please, please fix this on 1.3.
This is the sort of change that belongs at the beginning of an alpha milestone, not at the end of a release cycle.
Why? this aint an enhancement. this is a bug. It could well be considered for a major release version. Not to mention it is tagged for future and severity to minor. When are we going to see Mozilla swimming? Arent you guys bored of fixing web browser features that belongs to bronze age? I see tons of new features on 1.3b although this kinda bugs are all still tagged for an NP future. All the Iframe bugs are tagged future, for example. Is the purpose no longer having a next generation browser anymore? AOL version 9 pressure to drivers perhaps ;)?
> Why? Because it would be a complicated, delicate change with lots of potential for regressions? Thus not acceptable for a "stable" release without a _lot_ of prior testing. Drivers have nothing to do with anything. This is a volunteer project; people work on what they work on. The number of people who want to work on layout is very low, largely (in my opinion_ due to pushy bug reporters who demand things as their right without thinking about the fact that developers may have very limited time to work on Mozilla at all.... For your information,right now the focus of the few people working on layout is on basic arch work that will make the code simpler. Once that's done, fixing this bug should be a lot easier.
>Because it would be a complicated, delicate change with lots of potential for >regressions? Thus not acceptable for a "stable" release without a _lot_ of >prior testing. very well, >Drivers have nothing to do with anything. This is a volunteer project; people >work on what they work on. The number of people who want to work on layout is >very low, largely (in my opinion_ due to pushy bug reporters who demand things >as their right without thinking about the fact that developers may have very >limited time to work on Mozilla at all.... Yes it is a volunteer project. but for the few that is although there is nothing wrong with it and seems like has been working just fine. However, it has been quite a while since version 1.0,and I still dont see a serious attempt to clean up DOM/Layout level bugs by the Netscape engineers(not the volunteers). >For your information,right now the focus of the few people working on layout is >on basic arch work that will make the code simpler. Once that's done, fixing >this bug should be a lot easier. Uh I see now! Although we all thank you very much for all the effort and time you guys spent on Mozilla development, I regretfully see above quote as a serious problem. That certainly would require a lot of rewrite and reinterfacing in the spagetti(no offense) of Mozilla codebase. It gives me the signal that "future" tag meant for this bug and its alikes is really an unforeseeable future. In any case, I sincerely thank you all very much for the incredible effort put. regards, ps: how is the vacation going?
> I still dont see a serious attempt to clean up DOM/Layout level bugs by the > Netscape engineers You do know that Netscape pulled pretty much everyone off of layout, right? I think there is basically 1 person actively doing layout work at Netscape right now (the layout group has 4 people technically, but 3 of them are being distracted with other projects by management).
I came across this behavior recently, wondered if it was a bug, or not, and so went to check the spec. http://www.w3.org/TR/REC-CSS2/visudet.html#containing-block-details "If the element has 'position: absolute', the containing block is established by the nearest ancestor with a 'position' other than 'static', in the following way: 1. In the case that the ancestor is block-level, the containing block is formed by the padding edge of the ancestor. 2. In the case that the ancestor is inline-level, the containing block depends on the 'direction' property of the ancestor: 1. If the 'direction' is 'ltr', the top and left of the containing block are the top and left content edges of the first box generated by the ancestor, and the bottom and right are the bottom and right content edges of the last box of the ancestor. 2. If the 'direction' is 'rtl', the top and right are the top and right edges of the first box generated by the ancestor, and the bottom and left are the bottom and left content edges of the last box of the ancestor. If there is no such ancestor, the content edge of the root element's box establishes the containing block." In this case, the ancestor is not block-level, or inline-level. It is display: table-something. So are we in the "If there is no such ancestor, ..." clause? If we are, then Mozilla is currently doing the right thing, and this bug is INVALID. Of course, this may be an unitentional feature of the CSS2 spec, however they have not changed it in CSS2.1. Perhaps someone should check with the CSS2.1 people. http://www.w3.org/TR/CSS21/visudet.html#containing-block-details For reference, IE6 renders as if this bug was fixed, while Opera 7.03 reders this like Moz 2003030908 does (the alledgedly buggy behavior).
A table may not be 'display: block', but it is clearly still a block-level box. From http://www.w3.org/TR/REC-CSS2/tables.html: The following 'display' values assign table semantics to an arbitrary element: table (In HTML: TABLE) Specifies that an element defines a block-level table: it is a rectangular block that participates in a block formatting context.
A table may not be 'display: block', but it is clearly still a block-level box. From http://www.w3.org/TR/REC-CSS2/tables.html: The following 'display' values assign table semantics to an arbitrary element: table (In HTML: TABLE) Specifies that an element defines a block-level table: it is a rectangular block that participates in a block formatting context.
1. Tables are block-level. 2. for 'position: absolute', things are more complicated (CSS2 9.7) but this is still valid.
Resummarizing based on duplicates.
Summary: relative/absolute positioned table not abs pos containing block → positioned table or internal table elements not abs pos containing block
*** Bug 199444 has been marked as a duplicate of this bug. ***
Depends on: 203225
(In reply to comment #17) > 7624 // Is it positioned? > 7625 // If it's a table then ignore it, because for the time being tables > 7626 // are not containers for absolutely positioned child frames It's been there since the very beginning of that file. Looks like this is a seriously dated decision, possibly pre-CSS1.
Blocks: 257020
*** Bug 258736 has been marked as a duplicate of this bug. ***
Blocks: 258763
Blocks: 225429
*** Bug 266715 has been marked as a duplicate of this bug. ***
Wow... 12-28-2000... Don't hold my breath eh?
(In reply to comment #37) > Wow... 12-28-2000... Don't hold my breath eh? Please keep those remarks to yourself, this doesn't help.
*** Bug 268505 has been marked as a duplicate of this bug. ***
*** Bug 270741 has been marked as a duplicate of this bug. ***
*** Bug 273970 has been marked as a duplicate of this bug. ***
*** Bug 288794 has been marked as a duplicate of this bug. ***
How are advantages of this behavior? According my opinion is it only disadvantage. How much authors use this behavior? How many pages can change of this behavior cause regression? Actual situation, why Opera and Mozilla implements this variously, is problem for webdesigner which can use it. What about rounded corner in table block? I wanted use this in this page: http://tom.gonet.cz/projekty/seznamka/_test2/, but I can’�. This is the same situation as with <ul>, <li> behaviors of margin and padding in Mozilla and Opera. This is bad for community of web designers and users.
*** Bug 291009 has been marked as a duplicate of this bug. ***
Would it not suffice to remove && disp->mDisplay != NS_STYLE_DISPLAY_TABLE from /layout/base/nsCSSFrameConstructor.cpp:7807?
No, since the elements in question would need to be changed to support absolutely positioned children.
Blocks: 288351
This _very_ old bug still not fixed.
*** Bug 288351 has been marked as a duplicate of this bug. ***
Blocks: 217002
Is my marked-blocked bug 217002 a dupe of this one, or a derivative issue?
I would like this to be fixed as most others in this thread (save for the "engineering" issues, which are valid enough, but five years?).
*** Bug 321363 has been marked as a duplicate of this bug. ***
Hi all, I am celebrating today my third anniversary as a reporter of this bug. I reported it Dec 23, 2002 (http://bugzilla.mozilla.org/show_bug.cgi?id=186581)and was soon notified that it was a duplicate of this one. In just a few days the bug will pass its fifth anniversary since originally reported by Stephen Clouse on Dec. 28, 2000, near the end of the last century. I've subscribed to the "progress" of the bug over the years since then. Here are my picks for each year's highlights: o 2001: A comment by Hixie (https://bugzilla.mozilla.org/show_bug.cgi?id=63895#c4). I didn't understand this one really ( I don't understand any of the technical comments really) but it seems really cool that Hixie has been on the case. O 2002: Derek (https://bugzilla.mozilla.org/show_bug.cgi?id=63895#c16): "This seems like a pretty significant bug, considering that CSS2 positioning is supposed to end dependence on table-based layouts." o 2003: Boris (https://bugzilla.mozilla.org/show_bug.cgi?id=63895#c25) complaining about "pushy bug reporters who demand things as their right without thinking about the fact that ..." yada yada. o 2004: Joe (https://bugzilla.mozilla.org/show_bug.cgi?id=63895#c37) "Wow... 12-28-2000... Don't hold my breath eh?", followed by Martin's riposte (https://bugzilla.mozilla.org/show_bug.cgi?id=63895#c38) "Please keep those remarks to yourself, this doesn't help." o 2005: ATom (https://bugzilla.mozilla.org/show_bug.cgi?id=63895#c43): "How are advantages of this behavior? According my opinion is it only disadvantage. How much authors use this behavior? How many pages can change of this behavior cause regression?" I'm looking forward to lots more analysis and opinion on this bug in 2006! Happy holidays, Hugh
(In reply to comment #52) Hi Hugh, I can agree with you that this is a long standing bug. I attempted to help find a resolution in December of 2004 with 250763. Unfortunately, the developers seem to think this is an IE issue not a Mozilla issue....and that my report was fixed, but alas it was not and then became a duplicate of 63895. I find it disconcerting that when you follow the bug blocks that there are so many dependencies and duplicates being shown for the positioning that I believe the probelm lies in the fact that no one has really gathered all the pieces together to determine the real underlying principle to the cause. I've read several of the postings and followed many of the duplicates and there is one underlying principle to most: CSS compliance to positioning blocks does not seem to be happening properly. The behaviour does not go to the 'expected right'. This my perspective of the issue. I think we as a community need to rally all reporters to vote for the open bugs, blocks and dependencies.
Hi all, Just thought I'd log in to commemorate the seventh anniversary of this bug. I guess there's no progress to report, and there hasn't even been opinion expressed. (but thanks for your reply of last year, Shelley). I worked around this bug four years ago, and I guess everyone else did too. Regards, Hugh
It's a shame that a web standards bug that predates the release of Internet Epxlorer 6 is still not fixed...
Attached file Testcase —
I just went through the dependency list again and found these links to still be open and all related to positioning. Granted one is Firefox, but the underlying complaint is positioning of a block not going to the right of the parent. If I had more time, I would look at the other crossed out links. I would bet there are still other similiar links that are open. 19637 63895 217002 222220 203225 203229 243519 285871 314585 320865 340946
table is'nt only display type with this problem. The same problem is in display: inline-block elements, here is Example: http://atom.mamto.cz/projekty/opera/bug15/index2.html I tested also dev build of Firefox 3 which support inline-block but has the same problem with relative position. Why this is only minor problem I think, that this is very big problem. This work also in Explorer, and this is very usefull function for developer which use CSS layout.
I am having a problem that has the same results as test case 1. The only difference with my scenario is that I am not actually using a table but instead using a div with display:table; It seems that once you do that, the contents can no longer be positioned inside like they can when the containing element is display:block; It seems to me that the problem is that display:table; needs to still be recognized as a "block" element. My scenario actually works perfect in IE6 and IE7 which is usually not the case.
Actually my scenario seems to be ATom's test case exactly.
Copied from <http://lists.w3.org/Archives/Public/www-style/2007Sep/0187.html> (it seems to be reflected in Summary): On Wednesday 2007-09-26 21:31 -0500, Boris Zbarsky wrote: > Kornel Lesinski wrote: > >I think the latter: > > > >"The effect of 'position:relative' on table-row-group, > >table-header-group, table-footer-group, table-row, table-column-group, > >table-column, table-cell, and table-caption elements is undefined." > >http://www.w3.org/TR/CSS21/visuren.html#choose-position > > I don't recall that being there before. Must have been added so CSS21 can > actually exit CR... It was added in response to Issue #20 on the issues list for http://www.w3.org/TR/2003/WD-CSS21-20030915/ , which, given the lack of a URL, was presumably raised by Ian, and concerns what happens when the above elements get a stacking context (due to 'position:relative' or 'opacity') being undefined. -David
Blocks: 437722
(In reply to comment #34) > (In reply to comment #17) > > 7624 // Is it positioned? > > 7625 // If it's a table then ignore it, because for the time being tables > > 7626 // are not containers for absolutely positioned child frames > > It's been there since the very beginning of that file. Looks like this is a > seriously dated decision, possibly pre-CSS1. As far as I kan se this points directly to the problem (the decision must have been done later). borderCollapse: and position: was defined by CSS2 (12-May-1998). Table attributes BORDER, CELLSPACING and CELLPADDING defined in HTML3.2 combined with CSS2.borderCollapse: introduced very complex rules for sharing space occupied by common margins, border and padding, possibly resulting in fractional px values. position: absolute complicated position calculations for tables further. CSS2 did not define all implications of this and was probably the reason for the defensive decision above. 10 years has gone since and the need for advanced dynamic web-applications has increased considerably, causing the problem to grow in significance and need for reconsideration. Gecko’s implementation for absolute positioning also gives nice opportunities not in IE, eg. it is possible to move a parent without moving children. This facility is not defined in any CSS as far as I know? What should happen to an object when parent moves? Would have been nice to have the possibility in CSS to set ‘move_separate’ or ‘move_with_parent’ Sorry if this is considered ‘advocacy’ but I do`nt know where else to post it.
Hey fellas, its 2009 now and the 63895 bug is still going. Lets face it, if a web page viewer can not see the web page because its overlapped then there is a browser problem. (don't ya think?) I understand that Firefox is a volunteer project but I would have thought this would have been fixed 7 or 8 years ago. Do you think this will be fixed in the "near future" or should I go to crome or back to IE? Your call.
Just a note that I have encountered this bug in FF v3.0.10 while trying to implement a hover expansion of the contents of a table cell, like so: <td class="notworking"> <div class="note">blah yadda yadda </div> </td> with td.notworking { position: relative; width: 5em; height: 1em; } div.note { position: absolute; z-index: 0; width: 4em; } td.notworking:hover { top: -10em; left: -5em; z-index: 1; width: 40em; height: 20em; overflow-y: scroll; } [think I got that right-- untested] The hover works, sort of. But the position is relative to the div containing the entire table; it should be relative to the cell. A work-around with an added div in the cell gets what is intended-- at the cost of more mark-up and a fussier style sheet. I understand that the problem arose out of a design decision around 1999 or so, but with 10 years' advances in CSS and DOM, it would be good to revisit that decision.
Correction: s/td.notworking:hover/div.note:hover/
(In reply to comment #68) > <td class="notworking"> > td.notworking { > position: relative; > } The effect of 'position:relative' on table-row-group, table-header-group, table-footer-group, table-row, table-column-group, table-column, **table-cell**, and table-caption elements is undefined. (from http://www.w3.org/TR/CSS2/visuren.html#choose-position) This means your note is not a valid bug.
Re #70 (re #68): No. The bug is still there. That the W3C has not provided a standard behavior in this situation does not mean that the current behavior is excusable. There are two ways of resolving this bug that I can see: 1. either implement the intuitively obvious behavior, OR 2. make it so that any attempt to use relative positioning within the listed elements (see #70) breaks completely, in the same way that a divide by zero error completely breaks a mathematical expression. The current approach of simply ignoring a position:relative attribute in these instances is just plain wrong. That leads to a partial fail condition where instead of the styling breaking completely on the undefined attribute, it mysteriously works in a totally unexpected way by failing silently and passing directives up to the containing block. On reflection, all instances like this where a stylist might call for something that is explicitly undefined in the standards should result in a total failure of the styling. And possibly an alert box...
Assignee: layout.r-and-a-pos → nobody
QA Contact: madhur → layout.r-and-a-pos
(In reply to comment #74) > *** Bug 558665 has been marked as a duplicate of this bug. *** Sorry about the comment dudes but...10 years should be enough to fix such an irritating bug.
What an incredibly fun and interesting read this is. I've just posted this bug on the forums (http://forums.mozillazine.org/viewtopic.php?f=25&t=1956209&p=9675587), only to be told it was reported 10 years ago. I love the "anniversary" posts! :) Imagine that.. a Firefox hack, well I'll be. I'm sure Firefox devs are the first to talk about "ugly IE hacks", IE this, IE that.. well the shoe is firmly on the other foot here. Chrome, Safari, Opera, IE.. all behave the same, but Firefox needs to be different. Why don't the devs want to touch this one? Too boring? Surely can't be complicated. Anyway, welcome to the age of the Firefox hack!
(In reply to comment #76) > Imagine that.. a Firefox hack, well I'll be. I'm sure Firefox devs are the > first to talk about "ugly IE hacks", IE this, IE that.. well the shoe is firmly > on the other foot here. Chrome, Safari, Opera, IE.. all behave the same, but > Firefox needs to be different. Why don't the devs want to touch this one? Too > boring? Surely can't be complicated. > > Anyway, welcome to the age of the Firefox hack!
Ouch, I did not mean to send an email, just to add myself to the CC list. Anyway, since I got there... (In reply to comment #76) > Imagine that.. a Firefox hack, well I'll be. Do you have a working hack for when the relatively positioned ancestor is supposed to be a table cell?
tracking-fennec: --- → ?
As of today, 25 other bugs have been retired by reporting them as duplicates of this 10 year old bug. That suggests there is probably a lot more frustration over this problem than the level shown in these comments. Do the votes for fixing this one get incremented properly when another bug report is declared to be a duplicate of this one? Currently there are 49 votes to get this fixed, but should that number be at least 74 (assuming each of the other retired bug reports got at least one vote?)
tracking-fennec: ? → 2.0-
tracking-fennec: 2.0- → 2.0next+
tracking-fennec: 2.0next+ → 6+
(In reply to comment #71) > The bug is still there. That the W3C has not provided a standard behavior in > this situation does not mean that the current behavior is excusable. There are > two ways of resolving this bug that I can see: > > 1. either implement the intuitively obvious behavior, OR > > 2. make it so that any attempt to use relative positioning within the listed > elements (see #70) breaks completely I agree. (In reply to comment #78) > Do you have a working hack for when the relatively positioned ancestor is > supposed to be a table cell? I too am wondering what the workaround for this is until this gets fixed.
(In reply to comment #88) > I too am wondering what the workaround for this is until this gets fixed. Never mind, I found the workaround: <td> <div style="position:relative; height: 100%"> <div style="position: absolute; top: -25px; left: -25px;"> Content </div> </div> </td> I should also note that Chromium also seems to ignore position:relative for td's.... not that that makes this behavior correct.
(In reply to comment #71) > The bug is still there. That the W3C has not provided a standard behavior in > this situation does not mean that the current behavior is excusable. Afaik the W3C is planning to revamp the display properties and split them up into display-inside and display-outside in some future version of CSS. For that they'll probably have to re-specify how the containing block is determined too. That might finally give us some clear behavior for this particular problem.
(In reply to comment #89) > (In reply to comment #88) > > I too am wondering what the workaround for this is until this gets fixed. > > Never mind, I found the workaround: > <td> > <div style="position:relative; height: 100%"> > <div style="position: absolute; top: -25px; left: -25px;"> > Content > </div> > </div> > </td> That's hardly a workaround, because the for example vertical-align doesn't work in those (as it does in everything table-cell).
I'm curious how jQuery can work out the position of a table cell within a table cell, if you can't tell what's relative to it? http://pastebin.com/u1bJfAcz Still, it does, so that at least lets us position things relative to a cell, even if we have to use a javascript to do it.
tracking-fennec: 6+ → 7+
Depends on: 659828
Attached patch Reftest — — Splinter Review
My patches in bug 10209 and bug 659828 fix this. Here's the reftest based on the testcase in this bug.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #538160 - Flags: review?(roc)
Mark, any reason why this bug needs to be set to track fennec7?
Comment on attachment 538160 [details] [diff] [review] Reftest Review of attachment 538160 [details] [diff] [review]: -----------------------------------------------------------------
Attachment #538160 - Flags: review?(roc) → review+
(In reply to comment #89) > Never mind, I found the workaround: > <td> > <div style="position:relative; height: 100%"> > <div style="position: absolute; top: -25px; left: -25px;"> > Content > </div> > </div> > </td> This didn't work for me in Firefox 5. However, setting the outside div to display: inline did work. I.e: <td> <div style="position: relative; display: inline;"> <img style="position: absolute; bottom: 10px;" src=... /> </div> </td>
tracking-fennec: 7+ → +
(In reply to Ehsan Akhgari [:ehsan] from comment #93) > Created attachment 538160 [details] [diff] [review] [diff] [details] [review] > Reftest > > My patches in bug 10209 and bug 659828 fix this. Here's the reftest based > on the testcase in this bug. Those bugs landed, so this is now fixed. I've also landed the test: https://hg.mozilla.org/mozilla-central/rev/f59216ae6553
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: Future → mozilla10
Congratulations folks, and thanks, and I take back the snarks -- undeserved for sure. You've done a great job to keep tracking and pursuing this bug until you got it. Good work.
Actually, this isn't fixed as filed. This bug included rows, cells, captions, etc, not just tables; lots of the duplicates are about those. And those are not fixed...
In fact I think we should just reopen this bug, since it's tracking all the duplicates and dependencies for this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Summary: positioned table or internal table elements not abs pos containing block → positioned internal table elements not abs pos containing block
This is actually a bit of a pain to do for table cells and rows, because of the size changes they do _after_ reflow... :(
Severity: minor → normal
Priority: P4 → --
Target Milestone: mozilla10 → ---
Interesting problem. Will someone kindly explain to a web author why cells are handled differently from a % width <div> or a <div> sized with flexbox? Thank you!
(In reply to Boris Zbarsky (:bz) from comment #104) > This is actually a bit of a pain to do for table cells and rows, because of > the size changes they do _after_ reflow... :( They do that?! Wouldn't that break tons of other things? Also, should we use FinishAndReflowAbsoluteFrames here instead of ReflowAbsoluteFrames? Of course, depending on what really happens with those size changes, the answer might be no...
I apparently missed comment 106 and 107 somehow. :( > Will someone kindly explain to a web author why cells are handled differently Because they have to have totally different behavior, mostly. ;) > They do that?! Absolutely. Row vertical alignment does that. > Wouldn't that break tons of other things? Apparently not. In terms of how to implement this... It seems to me that the right thing to do is to do the reflow of absolutely positioned stuff hanging off cells and rows after layout of the table as a whole finishes. The only question is what happens for scrollable cells, assuming those are supported.
Didn't read all the comments (I know...) but I encountered the same bug and found a possible workaround: http://jsfiddle.net/delphiki/ZjqMF/7/
(In reply to Julien Villetorte from comment #109) > Didn't read all the comments (I know...) but I encountered the same bug and > found a possible workaround: http://jsfiddle.net/delphiki/ZjqMF/7/ That workaround doesn't seem to work... it doesn't place the span on the bottom right corner of EACH cell
(In reply to Aldi from comment #110) > (In reply to Julien Villetorte from comment #109) > > Didn't read all the comments (I know...) but I encountered the same bug and > > found a possible workaround: http://jsfiddle.net/delphiki/ZjqMF/7/ > > That workaround doesn't seem to work... it doesn't place the span on the > bottom right corner of EACH cell That's because only one cell is on display:inline-block.
Depends on: 746705
Attachment #669904 - Attachment mime type: text/plain → text/html
For reference: test of already fixed case when offsetParent is table itself; last broken in Firefox 3.6 (?), OK now. Problem remains in relatively positioned cells, rows, etc. What about making bug 35168 duplicate of this one?
Attachment #670295 - Attachment mime type: text/plain → text/html
This is definitely still a major bug. Just to put an extremely fine, and hopefully painful, point on just how bad this is, this bug was filed in the year 2000 and is still a problem. And even IE gets it right!
This bug is 12 years old. GET IT **** DONE!!!
Wow you people are a mess. This bug has been around for 12 years!! are you kidding? It works in IE!! I can't believe that Firefox is now the browser that I am having to code around.
a very clear example of this bug: http://jsfiddle.net/YjHaB/9/
I just found out about this bug today as I noticed my layout looked all messed up in Firefox. I made a small reduced test case [1] to prove it, but apparently that's not really necessary, seeing that you've known about this already bug back in Netscape times... ^^ Anyway, I also took a ran the test case on browsershots, shows the problem pretty clearly. [2] [1] http://labs.avd.io/bugz/table-bug.html [2] http://browsershots.org/http://labs.avd.io/bugz/table-bug.html
same testcase available at http://jsfiddle.net/3Dsd2/1/ 12 years, and still counting ...
Attachment #715427 - Attachment mime type: text/plain → text/html
Guys, please fix this problem, as it's breaking the web. I've run into this issue 6 or 7 times now over the past 4 years and the workarounds really stink - and they're only needed for THIS browser. Industry standards are often as important as W3C standards...
Guys, please fix this problem, as it's breaking the web. I've run into this issue 6 or 7 times now over the past 4 years and the workarounds really stink - and they're only needed for THIS browser. Industry standards are often as important as W3C standards...
Please fix this there really is no excuse for this.
Very frustrating. All the other browsers (including recent versions of IE) support this behaviour. Using ‘position’ to move table cells around is one thing, but using ‘position: relative’ (and ‘overflow:hidden’) to create a new positioning context works everywhere else, and makes many layout tasks easier. [I’m currently using a hack, ‘@-moz-document url-prefix() {…}’ to work around this for Firefox, the first time I’ve had to target Mozilla with a hack ever.]
Really? 13 years and still no solution? Mozilla, Stop making an OS, prettifying your GUI, and taking photos with your mascot and fix this bug! Really, it's little things like this that keep me believing that Google really knows what they're doing and Mozilla doesn't.
Is there any idea about how long will it take to have this issue fixed?
Alex: as it will celebrate it's 12.5 year anniversary in 4 days time, I wouldn't hold my breath. I seriously doubt this will ever really get fixed...
Hello I performed some tests and I found a workaround, at least for my condition. Inside the TD I added a DIV with position:relative and I moved all my INPUT to inside that DIV. This way the INPUTs respected the absolute positioning inside each TD and I was successful to reach the layout I need. I hope this will be helpful to others! Alex
I'm not very aware of many techniques that make vertical-align: middle usable than setting divs to display: table-cell. The layout I'm using in this codepen works as expected in Chrome and Safari: http://codepen.io/amboy00/pen/jyvBu IMO, the positioned elements in FF don't work as expected.
As for as I know, at some point the similar positioning problem occured in the old Flexbox implementation (display: -moz-box etc.). My tests show that in the new Flexbox implementation (Firefox 22+) there is no such problem, flexible boxes with position:relative serve as containing blocks for their positioned content quite well. Since flexible box model and table model have some similarities, can't the experience of implementing positioning in flexible boxes be used in fixing this annoying bug?
Blocks: 892669
(In reply to Josh S from comment #118) > this bug was filed in the year 2000 and is still a problem. Should we call it Firefox's Y2K bug?
You're kidding, right? It isn't funny. Probably I will be dead, when this bug will be fixed. What is "your tests"? Are you a preschool child? You can't read first sentence from first comment in this topic? I can do it for you http://jsfiddle.net/QtEgG/2/ Stop increasing versions. Fix bugs first!
Assignee: ehsan → seth
Firefox sucks.
No Steven, Firefox does not "suck" Without Firefox, we'd still have an IE monopoly with all kinds of blatant standards-ignoring goodness to deal with. For the most part Firefox does a fantastic job rendering things as they should... there's only a few minor bugs from time to time, and they generally fix them in a reasonable timeframe. This one is just an exception in an otherwise outstanding product.
This exception brings down the entire product. Data-driven apps need very dynamic tables and Firefox fails at it. IE is pretty good nowadays, thanks to Firefox or not.
It seems unreal that a bug like this could still be around after almost 13 YEARS! Yet another demo: http://codepen.io/FStop/pen/GebwD Fails in nightly as well. Is the really a hard one to fix? Or is it just not important enough?
Please fix this, or I may have to redirect my Firefox traffic to the Chrome (or even IE10, eww) download page. Thanks.
> Is the really a hard one to fix? Yes. It requires more or less completely redoing when absolutely positioned items are actually positioned... See comment 104.
(In reply to Boris Zbarsky [:bz] (Vacation Oct 12 - Oct 27) from comment #150) > > Is the really a hard one to fix? > > Yes. It requires more or less completely redoing when absolutely positioned > items are actually positioned... See comment 104. Well it's about time it was fixed, eh? Literally every other browser does this properly, and I'd be willing to bet that this very bug has changed web design paradigms to work around it.
This is relatively high on our priority list, but I can't make any promises about when it will be fixed. Sorry for failing to recognize more quickly when this bug transitioned from being a bug where we were the only major browser that implemented enough features for it to make sense to even report it (which led it to be relatively low priority) to a bug that all of the other major browsers had fixed (which makes it high priority now).
And here I come begging you to fix it.
Yes, this is really needed!
Shouldn't this bug be tagged with [parity:ie] [parity:Safari] [parity:Chrome] [parity:opera]?
blocking-b2g: --- → leo?
(In reply to Carlos Alén Silva from comment #155) > Shouldn't this bug be tagged with [parity:ie] [parity:Safari] > [parity:Chrome] [parity:opera]? If "parity" means the other browser doesn't have this bug, then yes. However, as far as I know, it means it's the same on other browsers. Just adding my 2 cents
This doesn´t happen in Chrome or IE8+. That´s why I filed the issue.
Comment on attachment 538160 [details] [diff] [review] Reftest [Triage Comment]
Attachment #538160 - Flags: approval-mozilla-b2g26+
Attachment #538160 - Flags: approval-mozilla-b2g18+
Attachment #538160 - Flags: approval-mozilla-b2g26+
Attachment #538160 - Flags: approval-mozilla-b2g18+
This patch allows all internal table objects other than 'table-column's and 'table-column-group's to serve as absolute containing blocks for their descendants. This is implemented by tracking positioned internal table objects during frame construction and reflowing their absolutely positioned descendants at the end of the table reflow process.
Attachment #8368705 - Flags: review?(dbaron)
These are positioning tests for all of the internal table objects that can now serve as absolute containing blocks.
Attachment #8368706 - Flags: review?(dbaron)
Haven't had a chance to look much, but what does nsIFrame::eCanContainOverflowContainers have to do with this?
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #161) > Haven't had a chance to look much, but what does > nsIFrame::eCanContainOverflowContainers have to do with this? Without adding that bit, I experienced assertion failures and crashes when the absolutely positioned child ended up outside its parents.
Overflow containers are related to pagination of overflowing out-of-flows (floats, abs pos). So is stuff on those lists correctly managed, reflowed, and painted?
Oops, I see how it's related; it's worth making sure that you're doing all the things expected of frames that support overflow containers. I'm not sure off the top of my head what those are. That might be complicated enough that it's worth a separate patch or bug, but really not sure.
Blocks: 967870
Two questions about part 1: 1) Don't we need to unregister things if the child frames get destroyed before the table does? 2) Why are we checking whether the _parent_ supports transforms if the child is transformed?
Comment on attachment 8368705 [details] [diff] [review] (Part 1) - Support internal table objects as absolute containing blocks. >+ nsHTMLReflowState reflowState(aPresContext, this, >+ aReflowState.rendContext, >+ nsSize(size.width, size.height), >+ nsHTMLReflowState::DUMMY_PARENT_REFLOW_STATE); You almost certainly want NS_UNCONSTRAINEDSIZE rather than size.height; that seems likely to be why you're getting overflow containers (and incorrectly paginating). I suspect once that's fixed, doing pagination correctly can probably go to a separate bug.
(In reply to Boris Zbarsky [:bz] from comment #165) > 1) Don't we need to unregister things if the child frames get destroyed > before the table does? Yeah, we do. I originally had code in there to do that but somehow it didn't make it into the patch. =\ > 2) Why are we checking whether the _parent_ supports transforms if the > child is transformed? Sounds like you're referring to the code in nsCSSFrameConstructor? Probably just a mistake. I'll take a look when I get a chance.
> Sounds like you're referring to the code in nsCSSFrameConstructor? Yes.
This updated patch should address all of the concerns that have been raised so far. It seems probable to me that the vast majority of the time, internal table objects will be getting destroyed at the same time as the table that contains them (for example, when we navigate away from a page) so I went ahead and optimized for that case. If the table itself is getting torn down, we don't bother updating the positioned objects list and let the table clear it out when it gets destroyed. This lets us elide a lot of potentially unnecessary mutation of the list.
Attachment #8372016 - Flags: review?(dbaron)
Attachment #8368705 - Attachment is obsolete: true
Attachment #8368705 - Flags: review?(dbaron)
Per today's discussion, make sure that we only store these properties on the first continuation frame of the nsTableFrame.
Attachment #8372711 - Flags: review?(dbaron)
Attachment #8372016 - Attachment is obsolete: true
Attachment #8372016 - Flags: review?(dbaron)
Ah yes. That reminds me that I need to fix the author field on these patches.
Attachment #8372720 - Flags: review?(dbaron)
Attachment #8372711 - Attachment is obsolete: true
Attachment #8372711 - Flags: review?(dbaron)
Same here. Sorry for bugspam.
Attachment #8372721 - Flags: review?(dbaron)
Attachment #8368706 - Attachment is obsolete: true
Attachment #8368706 - Flags: review?(dbaron)
Add some helper methods to retrieve both visual and scrollable overflow areas in the frame's local coordinate space. This is needed to get correct overflow areas in part 2. Without it, we get some oranges due to asserts at nsFrame.cpp:6922 that "Computed overflow area must contain frame bounds", because GetOverflowAreas returns the overflow areas *after* CSS transforms are applied.
Attachment #8373778 - Flags: review?(dbaron)
This new part 2 (previously part 1) is updated to use the new helper methods.
Attachment #8373779 - Flags: review?(dbaron)
Attachment #8372720 - Attachment is obsolete: true
Attachment #8372720 - Flags: review?(dbaron)
Just bumped the part number on this part.
Attachment #8373780 - Flags: review?(dbaron)
Attachment #8372721 - Attachment is obsolete: true
Attachment #8372721 - Flags: review?(dbaron)
Here's a new try job. The new patches I pushed just now should have taken care of all of the oranges revealed by the last try push, so hopefully this is nice and green. https://tbpl.mozilla.org/?tree=Try&rev=9b5fb38c3b32
Comment on attachment 8373778 [details] [diff] [review] (Part 1) - Add helper methods for retrieving overflow areas in the frame's local coordinate space. Could you make GetOverflowAreasRelativeToSelf look like GetScrollable/VisualOverflowRectRelativeToSelf instead of calling them, since then it will only do the property lookup once? r=dbaron with that
Attachment #8373778 - Flags: review?(dbaron) → review+
Do you still need the eCanContainOverflowContainers changes in this patch (part 2)? I'd have expected those to be in bug 967870.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #179) > Do you still need the eCanContainOverflowContainers changes in this patch > (part 2)? I'd have expected those to be in bug 967870. I don't think so; I'll upload a new version without them. By the way, this patch is leaking on some reftest, as you can see the from the try job. (Looks like it's leaking the nsTArray, which is to say, it's leaking the frame property. I'm very confused as to how that can happen.) I'm still tracking that down. It will get addressed before checkin.
Per comment 179, removed the eCanContainOverflowContainers changes.
Attachment #8375990 - Flags: review?(dbaron)
Attachment #8373779 - Attachment is obsolete: true
Attachment #8373779 - Flags: review?(dbaron)
Addressed the review comments. GetOverflowAreasRelativeToSelf will now do only one property lookup.
Attachment #8373778 - Attachment is obsolete: true
This version of the patch fixes the leak and renames things so that "internal table objects" is replaced by "table parts".
Attachment #8376603 - Flags: review?(dbaron)
Attachment #8375990 - Attachment is obsolete: true
Attachment #8375990 - Flags: review?(dbaron)
OK, all known issues have been addressed, so unless more problems are uncovered during review I'd say we're good to go here. Just to be sure (and in particular to verify the leak is gone), here's one more try job. https://tbpl.mozilla.org/?tree=Try&rev=8a23d399de9e
Comment on attachment 8376603 [details] [diff] [review] (Part 2) - Support table parts as absolute containing blocks. nsCSSFrameConstructor: The code in nsCSSFrameConstructor should have comments about why it's setting the NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN more conservatively than other code that sets that bit -- in particular, because there's a registration/unregistration cost to having that bit set, so you only set it when the style says that it's a containing block, rather than only based on the frame's capability to have the child list. (I'm really not sure why the other code isn't more conservative, though.) I say this because the comment would warn somebody trying to "clean it up" later. The repeated pattern: aDisplay->HasTransformStyle() && newFrame->IsFrameOfType(nsIFrame::eSupportsCSSTransforms) can be replaced with aDisplay->HasTransform(newFrame) (It wouldn't be unreasonable to assert that styleContext->GetStyleDisplay() == aDisplay as well. I'm sure assuming it is, and it appears HasTransform also asserts it.) nsTableFrame: Could you change aMustPassThru -> aMustPassThrough ? In GetTableFramePassingThrough, you can (and I think should) drop the boolean hitPassThruFrame by swapping the order of the two ifs inside the loop, and just returning null if you hit the pass-through frame. Simpler code, and less looping. >+typedef nsTArray<nsIFrame*> nsIFrameArray; Please call this FrameTArray; things starting with nsI look like interfaces. >+ auto positionedObjs = static_cast<nsIFrameArray*>(aPropertyValue); I don't think we can use auto yet, though I might be wrong. I suppose if it passed try on all platforms then maybe we can? There's a cleaner way to do this: >+ FramePropertyTable* props = tableFrame->PresContext()->PropertyTable(); >+ void* positionedPartsProp = >+ props->Get(tableFrame, nsTableFrame::PositionedTablePartArray()); ... >+ props->Set(tableFrame, >+ nsTableFrame::PositionedTablePartArray(), >+ positionedParts); which is (using FrameProperties, which is a thin wrapper that does this): FrameProperties props = tableFrame->Properties(); void* positionedPartsProp = props.Get(PositionedTablePartArray()); ... props.Set(PositionedTablePartArray(), positionedParts); Similar in UnregisterPositionedTablePart. >+ // If there are any positioned table parts, we need to reflow their >+ // absolutely-positioned descendants now that their dimensions are final. "positioned table parts" -> "relatively-positioned table parts", since there's no such thing as absolutely-positioned table parts, and so it makes things a little clearer. FixupPositionedTableParts: >+ FramePropertyTable* props = PresContext()->PropertyTable(); >+ void* positionedPartsProp = >+ props->Get(this, nsTableFrame::PositionedTablePartArray()); >+ auto positionedParts = static_cast<nsIFrameArray*>(positionedPartsProp); This should be just one line: auto positionedParts = static_cast<FrameTArray*>(Props().Get(PositionedTablePartArray()); >+ if (positionedParts) { Instead of indenting, return early if (!positionedParts). >+ for (size_t i = 0 ; i < positionedParts->Length() ; ++i) { Local style has no space before either semicolon. >+ auto positionedPart = static_cast<nsFrame*>(positionedParts->ElementAt(i)); Why the cast? I'd drop it and just do: nsIFrame* positionedPart = positionedParts->ElementAt(i); If you need a cast somewhere later, do that explicitly; this code makes it look to a casual reader like you're dealing with a container type that holds void*, which you're not. >+ reflowState.mCBReflowState = &aReflowState; It seems odd to set mCBReflowState on a reflow state with a null parent. Is this necessary? >+ // Reflow absolutely-positioned descendants of the positioned part. >+ overflowTracker.AddFrame(positionedPart); It probably makes sense to do this only if the overflow area has changed. Could you add a "FIXME:" comment pointing out that both the NS_UNCONSTRAINEDSIZE for the height and ignoring any change to the reflow status aren't actually correct (and possibly also the reflow status's initial value), and mean we'll never paginate these absolutely-positioned frames. It might not be a bad idea if bz has a quick look at the frame constructor changes. r=dbaron with the above changes, though
Attachment #8376603 - Flags: review?(dbaron)
Attachment #8376603 - Flags: review+
Attachment #8376603 - Flags: feedback?(bzbarsky)
Comment on attachment 8373780 [details] [diff] [review] (Part 3) - Add tests for positioned internal table objects serving as absolute containing blocks. Maybe rename all the table-* to in-table-*, because you're testing positioning in a table-* rather than of it? I'm a little skeptical of the value of the prose descriptions in the tests; they make it seem like that's a sufficient pass condition -- do you really think the prose conditions are sufficient? If not, maybe remove them... although I know the CSS WG likes them (though they're not otherwise in CSS WG format)? (If there were a spec that said what should happen here, I'd have suggested adding the relevant CSS WG metadata and putting the tests in layout/reftests/w3c-submitted/ instead... but there isn't.) r=dbaron on these, although I was expecting to see more coverage of right and bottom (with either auto width/height or auto top/left, although the former kills two birds with one stone). I think you need some test coverage of that as well. (Or am I missing it?)
Attachment #8373780 - Flags: review?(dbaron) → review+
Comment on attachment 8376603 [details] [diff] [review] (Part 2) - Support table parts as absolute containing blocks. The frame constructor changes look fine to me, modulo the existing comments.
Attachment #8376603 - Flags: feedback?(bzbarsky) → feedback+
Thanks for the review! I'll upload a new patch shortly with these issues addressed. A few comments below: (In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #186) > I don't think we can use auto yet We can! See here: https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code > >+ reflowState.mCBReflowState = &aReflowState; > > It seems odd to set mCBReflowState on a reflow state with a null parent. > > Is this necessary? Sadly (since I agree it'd be preferable to avoid it), it does seem to be necessary. Tons of assertions are generated without it. > >+ // Reflow absolutely-positioned descendants of the positioned part. > >+ overflowTracker.AddFrame(positionedPart); > > It probably makes sense to do this only if the overflow area has changed. I'm surprised that the overflow tracker doesn't do the right thing automatically! Will double check.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #186) > Comment on attachment 8376603 [details] [diff] [review] > In GetTableFramePassingThrough, you can (and I think should) drop > the boolean hitPassThruFrame by swapping the order of the two ifs > inside the loop, and just returning null if you hit the pass-through > frame. Simpler code, and less looping. Heh, this is a good point. Looking back at the code, all of the stuff you mention existed just to support the assert at the bottom of the function. =)
(In reply to Seth Fowler [:seth] from comment #189) > > >+ reflowState.mCBReflowState = &aReflowState; > > > > It seems odd to set mCBReflowState on a reflow state with a null parent. > > > > Is this necessary? > > Sadly (since I agree it'd be preferable to avoid it), it does seem to be > necessary. Tons of assertions are generated without it. ... Or not. I just checked and it isn't needed. This was probably another issue that was solved by switching to NS_UNCONSTRAINEDSIZE for the reflow state available height.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #187) > Comment on attachment 8373780 [details] [diff] [review] > Maybe rename all the table-* to in-table-*, because you're testing > positioning in a table-* rather than of it? Well, the names of the tests are just the names of the display values/frame types that are being tested. > I'm a little skeptical of the value of the prose descriptions in the > tests; they make it seem like that's a sufficient pass condition -- do > you really think the prose conditions are sufficient? If not, maybe > remove them... although I know the CSS WG likes them (though they're > not otherwise in CSS WG format)? Well, I agree that they're not really a sufficient pass condition, but the tests are designed in such a way that the most likely failure mode (the abs-pos child is positioned relative to the wrong absolute containing block) will cause the failure that the text describes. It's good to know for you're not a big fan of this approach, as it took more work to do it this way (particularly while sharing the references between the tests for the different table parts). However, since I've already done that work, I'm inclined to leave the text in. =) > r=dbaron on these, although I was expecting to see more coverage of > right and bottom (with either auto width/height or auto top/left, > although the former kills two birds with one stone). I think you need > some test coverage of that as well. (Or am I missing it?) The reason I didn't write tests that involved right and bottom was because I thought of these tests as testing two issues: (1) Is the abs-pos child positioned relative to the correct absolute containing block? (2) Is the abs-pos child positioned relative to the *final position* of the correct absolutely containing block, after all table reflow passes are complete? It seemed to me that usage of right/bottom wasn't relevant to those issues, and there were already a lot of tests. Do you think that coverage of right and bottom *is* relevant to those issues? Or am I just looking at the scope of these tests too restrictively? I'll needinfo you and if necessary add a part 4 with additional tests.
Flags: needinfo?(dbaron)
(In reply to Seth Fowler [:seth] from comment #192) > Well, I agree that they're not really a sufficient pass condition, but the > tests are designed in such a way that the most likely failure mode (the > abs-pos child is positioned relative to the wrong absolute containing block) > will cause the failure that the text describes. Actually, in terms of the two issues described in the later part of comment #192, it's also true that issue 2 will cause the failure that the text describes in those tests where the problem could occur.
Addressed review comments. The only change I made here that is not discussed in the review comments above is that I pulled the repeated code for registering positioned table parts into a separate static function. After the additional comments were added it started to feel like too big and perhaps too subtle a block of code to be repeated.
Attachment #8376603 - Attachment is obsolete: true
When I went to check this in I discovered it needed a rebase.
Attachment #8378650 - Attachment is obsolete: true
Attachment #8375992 - Flags: checkin+
Attachment #8378656 - Flags: checkin+
Attachment #8373780 - Flags: checkin+
Attachment #566883 - Attachment is obsolete: true
Attachment #538160 - Flags: checkin+
Sadface! The problem was that nsCSSFrameConstructor was creating continuation frames that had NS_FRAME_CAN_HAVE_ABSPOS_CHILDREN set (because they snarfed it from their previous continuation, via nsFrame::Init) but *weren't actually registered* with nsTableFrame. Will upload a patch shortly that fixes this. I've verified the fix locally, but will push another try job to make sure everything has shaken out.
Flags: needinfo?(seth)
Boris, sorry to add to your review load, but would you mind giving the frame construction changes another look? In particular, the changes to CreateContinuingTableFrame and CreateContinuingFrame are totally new in this version of the patch.
Attachment #8378801 - Flags: review?(bzbarsky)
Attachment #8378656 - Attachment is obsolete: true
Try job for the new patch here: https://tbpl.mozilla.org/?tree=Try&rev=b97a4a3d8daa
Attachment #8378801 - Flags: feedback?(dbaron)
Flags: needinfo?(dbaron)
(In reply to Seth Fowler [:seth] from comment #192) > > r=dbaron on these, although I was expecting to see more coverage of > > right and bottom (with either auto width/height or auto top/left, > > although the former kills two birds with one stone). I think you need > > some test coverage of that as well. (Or am I missing it?) > > The reason I didn't write tests that involved right and bottom was because I > thought of these tests as testing two issues: > > (1) Is the abs-pos child positioned relative to the correct absolute > containing block? > (2) Is the abs-pos child positioned relative to the *final position* of the > correct absolutely containing block, after all table reflow passes are > complete? > > It seemed to me that usage of right/bottom wasn't relevant to those issues, > and there were already a lot of tests. > > Do you think that coverage of right and bottom *is* relevant to those > issues? Or am I just looking at the scope of these tests too restrictively? I think it is relevant, in that the containing block has four edges, and some of those edges might not move in the later stages of reflow, and it's worth having some test coverage of the right/bottom edges of the containing block.
Comment on attachment 8378801 [details] [diff] [review] (Part 2) - Support table parts as absolute containing blocks. r=me. Sorry for the lag. :(
Attachment #8378801 - Flags: review?(bzbarsky) → review+
Comment on attachment 8378801 [details] [diff] [review] (Part 2) - Support table parts as absolute containing blocks. >+ // If there are any relatively-positioned table parts, we need to reflow their >+ // Retrieve the table frame, and ensure that we hit aMustPassThrough on the way. watch the 80th column violations. Otherwise looks good.
Attachment #8378801 - Flags: feedback?(dbaron) → feedback+
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #204) > Comment on attachment 8378801 [details] [diff] [review] > (Part 2) - Support table parts as absolute containing blocks. > > >+ // If there are any relatively-positioned table parts, we need to reflow their > > >+ // Retrieve the table frame, and ensure that we hit aMustPassThrough on the way. > > watch the 80th column violations. > > Otherwise looks good. What do you mean "looks good"? Was this put out in Nightly? It's still broken for me.
It will be in nightlies generated when the bug is marked fixed, which it is not now.
Marking as leave-open until the final set of tests gets in.
Keywords: leave-open
Attachment #8378801 - Flags: checkin+
If we're not sure we'll land the tests for 30, might be good to spin that off to a separate bug.
Keywords: relnote
(In reply to Boris Zbarsky [:bz] (reviews will be slow; ask someone else) from comment #210) > If we're not sure we'll land the tests for 30, might be good to spin that > off to a separate bug. I currently don't see any reason why we shouldn't, but I agree that a separate bug is appropriate if it looks like that might happen.
Depends on: 980223
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #186) > In GetTableFramePassingThrough, you can (and I think should) drop > the boolean hitPassThruFrame by swapping the order of the two ifs > inside the loop, and just returning null if you hit the pass-through > frame. Simpler code, and less looping. Doh, this is not actually equivalent, though I didn't immediately see the problem. This actually ends up returning null if we hit the pass-through frame, which is exactly the opposite of the intended behavior! I've reverted this change, which will fix bug 980223, in my local version of the patch. Will upload a new version shortly.
This makes the change mentioned in comment 212, resolving bug 980223. (Though in some sense it's resolved anyway by the backout...)
Attachment #8378801 - Attachment is obsolete: true
(Sorry, I intended to provide this as a separate patch, but it ended up being trickier than I thought due to some Mercurial wonkiness.) In addition to the changes mentioned in comment 212, this version of part 2 adds a warning during frame construction when we see a non-cell table part with relative positioning applied. The warning only appears in DEBUG builds (which, to my knowledge, includes Nightly) so it should be very low-impact. The relevant code is in nsTableFrame::RegisterPositionedTablePart.
Attachment #8387996 - Flags: review?(dbaron)
Attachment #8387244 - Attachment is obsolete: true
Fixed some issues that came up in offline discussion with dbaron. I no longer wrap the warning code in #if DEBUG since Nightly does not set -DDEBUG. (And it's not clear we want Nightly-only anyway.) Instead, to minimize the performance impact, I added a flag to the nsPresContext to ensure that we only report the error once. I experimented with setting the category argument to ReportToConsole to different values, including "CSS", but it made no difference; the web console always categorizes the message under JS. I tracked down this problem and filed bug 981241, but I don't think it needs to block this bug.
Attachment #8388006 - Flags: review?(dbaron)
Attachment #8387996 - Attachment is obsolete: true
Attachment #8387996 - Flags: review?(dbaron)
Comment on attachment 8388006 [details] [diff] [review] (Part 2) - Support table parts as absolute containing blocks. Maybe change "Your site" to "This site" in the error message? r=dbaron
Attachment #8388006 - Flags: review?(dbaron) → review+
Replaced "Your site" with "This site". Also, moved the error message's location string back to the layout properties file, since it looks like bug 981241 will make this nice very soon.
Attachment #8388006 - Attachment is obsolete: true
(In reply to Seth Fowler [:seth] from comment #218) > Also, moved the error message's > location string back to the layout properties file, since it looks like bug > 981241 will make this nice very soon. The category and the properties file used aren't related, though. But it seems like a reasonable change anyway.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #219) > The category and the properties file used aren't related, though. But it > seems like a reasonable change anyway. I thought it might be more intuitive for people looking for the string, but yeah, it didn't really matter.
Attachment #8388835 - Flags: checkin+
Writing the tests in part 4 uncovered another bug - OverflowChangedTracker does not propagate overflow all the way to the subtree root (the table frame), but only to its children, contrary to my assumption. This caused us to fail to render absolutely positioned children of relatively positioned table parts when they ended being placed above or to the left of the table frame. This patch fixes this problem by updating the overflow areas of the table frame after we apply the relative positioning. This will need to be uplifted to Aurora once it sticks to m-c.
Attachment #8392651 - Flags: review?(dbaron)
This patch adds additional tests which cover positioning by 'bottom' and 'right'. This complements the previous set of tests, which cover positioning by 'top' and 'left'. This will need to be uplifted to Aurora once it sticks to m-c.
Attachment #8392652 - Flags: review?(dbaron)
Here's a try job for parts 4 and 5 against current m-c: https://tbpl.mozilla.org/?tree=Try&rev=ff773525fa1b
Removed leave-open because parts 4 and 5 represent all of the remaining work for this bug.
Keywords: leave-open
Comment on attachment 8392651 [details] [diff] [review] (Part 4) - Propagate overflow of relatively positioned table parts to the table frame. >+ >+ // Update our own overflow areas. (OverflowChangedTracker doesn't update the >+ // subtree root itself.) >+ nsLayoutUtils::UnionChildOverflow(this, aDesiredSize.mOverflowAreas); Before you do this, you should do aDesiredSize.SetOverflowAreasToDesiredBounds(), because you want to discard the previous unioning of the children. This is testable, for example, with: <td style="position:relative; top: -50px"> <div style="height: 50px"> <div style="height: 100px"> </div> </div> </td> and testing that the table with this td at the bottom doesn't cause a scrollbar to scroll to where the cell would have been if it hadn't been relatively positioned. (I believe that's consistent with the way we treat overflow and relative positioning elsewhere, but it's worth checking before following my advice.) r=dbaron with that
Attachment #8392651 - Flags: review?(dbaron) → review+
Comment on attachment 8392652 [details] [diff] [review] (Part 5) - Add more tests for positioned internal table objects serving as absolute containing blocks. Looks like some of the history is recorded as copies and some isn't; is that intentional? Maybe more should be recorded as hg copies? (I think hg cp -A will work.) r=dbaron
Attachment #8392652 - Flags: review?(dbaron) → review+
Looked into dbaron's suggestion in comment 227 and it makes sense to me. Updated the patch.
Attachment #8392651 - Attachment is obsolete: true
(In reply to David Baron [:dbaron] (needinfo? me) (UTC+8, but slow response this week) from comment #228) > Looks like some of the history is recorded as copies and some isn't; is that > intentional? Maybe more should be recorded as hg copies? (I think hg cp -A > will work.) `hg cp -A` doesn't seem to work with patch queues, sadly. Thanks for the reviews! Looks like this is ready to go.
Attachment #8393294 - Flags: checkin+
Attachment #8392652 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/0320aabdedc5 https://hg.mozilla.org/mozilla-central/rev/d2bc5ad54215
Status: REOPENED → RESOLVED
Closed: 13 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Target Milestone: mozilla31 → mozilla30
Comment on attachment 8393294 [details] [diff] [review] (Part 4) - Propagate overflow of relatively positioned table parts to the table frame. [Approval Request Comment] Bug caused by (feature/regressing bug #): 63895 User impact if declined: Rendering and invalidation errors when relative positioning is used with table parts. Since this is a new feature in FF, it'd be preferable to avoid ever exposing web content to these problems. Testing completed (on m-c, etc.): On m-c for a day with no reported issues. Risk to taking this patch (and alternatives if risky): Low risk. This is a very small patch that is unlikely to introduce new issues. String or IDL/UUID changes made by this patch: None. (Note that I understand I don't need to request approval for test-only patches. I'll be uplifting part 5, which contains the tests for this issue, along with this patch.)
Attachment #8393294 - Flags: approval-mozilla-aurora?
TM tracks trunk landing. Status flags track branch uplifts.
status-firefox30: --- → affected
status-firefox31: --- → fixed
Target Milestone: mozilla30 → mozilla31
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #234) > TM tracks trunk landing. Status flags track branch uplifts. Fair enough, although 3/5 of this bug landed on 30. =)
Yeah, hence comment 210... It was a suggestion to prevent that sort of confusion.
Attachment #8393294 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Reproduced the issues on Nightly 2009-10-15. Verified fixed using the testcases on 31.0a1 (2014-03-25), win 7 x64.
Status: RESOLVED → VERIFIED
status-firefox31: fixed → verified
(In reply to Boris Zbarsky [:bz] from comment #236) > Yeah, hence comment 210... It was a suggestion to prevent that sort of > confusion. Yeah, in retrospect I should've done that. I didn't expect the tests to reveal new issues to be debugged, but that was naive. =\
Blocks: 1014506
No longer blocks: 1014506
Depends on: 1027611
Blocks: 1790067
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: