Closed Bug 73586 Opened 20 years ago Closed 13 years ago

matching of :first-child, :only-child, and :last-child not dynamically updated [SELECTORS-DYNAMIC]


(Core :: CSS Parsing and Computation, defect, P1)






(Reporter: glazou, Assigned: dbaron)



(4 keywords, Whiteboard: [Hixie-P3] important DOM1 bug (py8ieh:need tests))


(2 files, 1 obsolete file)

the attached testcase offers two links. Clicking on the first one will insert a
paragraph as first child of body. That paragraph should get the style from

  p:first-child { border : thick green solid }

A second link appends a paragraph to the children of body and the following rule
applies to it :

  p:last-child { border : thick red solid }

*** NOTE : this last rule only running if bug 42916 checked in ***

Inserting new paragraphs at the beginning or at the end of body should make the
paragraph who had the green or red border loose that border in favor of the
newly created one. It does no work.
Attached file DHTML test case (obsolete) —
Depends on: 46916
I think dan means bug 46916 (last-child) not bug 42916?
yes, 46916 ; sorry.
I don't see how this has anything to do with reflow.  Isn't it just that we
don't do enough dynamic re-styling when the content model is changed?  As we
add new selectors, we're probably going to run into more problems like this
unless we do more re-resolution.  (I wrote a comment to myself in
nsCSSFrameConstructor::ContentInserted that it may not re-resolve enough for
the CSS2 '+' combinator, although I never had a chance to test that.  Would
that be the right place to do it?  How much of a performance cost would such
changes be?  What optimizations could we make to avoid such a cost (particularly
for :subject)?)
Keywords: qawanted
Whiteboard: [Hixie-P3] important DOM1 bug (py8ieh:need tests)
Keywords: qawanted
Summary: inserting an element in the DOM does not generate reflow → matching of :first-child and :last-child not dynamically updated
Target Milestone: --- → mozilla1.0
The "+" issue is bug 110972.
Target Milestone: mozilla1.0 → mozilla1.2
Bulk moving Moz1.2 bugs to future-P2. I will pull from this list when scheduling
work post Mozilla1.0
Priority: -- → P2
Target Milestone: mozilla1.2 → Future
Summary: matching of :first-child and :last-child not dynamically updated → matching of :first-child and :last-child not dynamically updated [SELECTORS-DYNAMIC]
cc'ing myself
Assigning pierre's remaining Style System-related bugs to myself.
Assignee: pierre → dbaron
Priority: P2 → P1
Keywords: mozilla1.0
*** Bug 192151 has been marked as a duplicate of this bug. ***
Bug 73586, bug 98997, and bug 229915 would be fixed by what I described in step
3 of bug 15608 comment 28:

  When adding/removing content, just reresolve style on the parent (to handle
  empty, +, and all the new CSS3 variants).  (Perhaps there are good ways to
  optimize this, but it might just not be that bad.)
The problem is that that would be O(N^2) for long pages during load.

A more detailed approach might look something like this:
 * maintain a hashtable in nsCSSStyleSheet.cpp much like the one used for event
state selectors or attribute selectors, hashing all selectors with any of these
"child-counting" selectors (in which I include :empty).  There would also be a
global variable(s) to indicate whether any nth-last, last-of-type, or
nth-last-of-type were used.  This would probably allow for the necessary
optimizations.  (Consider that we use :-moz-last-node extensively in quirk.css,
and this actually leads to real bugs due to this bug.)

I should probably describe this in a little more detail at some point, or just
implement it.

It would be pretty easy to fix bug 73586 with this type of approach, but bug
229915 would probably be a little more work.  I need to think about that.
Attachment #28902 - Attachment is obsolete: true
Some other approaches might be feasible now that
nsIDocumentObserver::ContentReplaced no longer exists (which simplifies the
invariants for observer code).
*** Bug 254598 has been marked as a duplicate of this bug. ***
I was just wondering how this bug is being fixed.  Is it planned to be in for
Firefox 1.1?  Or is it for a latter date?
*** Bug 303775 has been marked as a duplicate of this bug. ***
This bug also applies to :only-child. Could someone change the summary accordingly?
boris/david, should I file a new bug re: c#19; or morph this one to include it, I am unsure exactly how similar the issue is in your mind(s).
Summary: matching of :first-child and :last-child not dynamically updated [SELECTORS-DYNAMIC] → matching of :first-child, :only-child, and :last-child not dynamically updated [SELECTORS-DYNAMIC]
Blocks: 357811
This bug affects not only pages that change dynamically; it also affects pages that are simply slow to load. The :last-child CSS rule is (wrongly) applied to all elements that were the last child at some point in the middle of rendering an unfinished page, even if additional elements followed it later.
QA Contact: ian → style-system
So I think I have a strategy for fixing this bug, bug 98997, and bug 229915.  We should set bits on nsINode (which means we need to mark some more nsINode flags an not reserved for implementations) that mean:
 1. element has an :empty or :-moz-only-whitespace selector
 2. arbitrary operations on children of the element (including append) require a reresolve on the element
 3. append requires a reresolve on the element's last child; other operations on children of the element require a reresolve of the element
 4. append requires no changes, but other operations on children of the element require a reresolve of the element

I'm not sure if we really need the distinction between (3) and (4).  We need (1) separate because of the pile of :-moz-only-whitespace rules in quirk.css -- this would be much easier if we moved that margin collapsing quirk into C++.

SelectorMatches and SelectorMatchesTree would then set these bits *any* time they tried to match one of the problematic selectors (match or no).  This will guarantee that we'll always have them set if the change might matter.  We should move the problematic selectors to the end of SelectorMatches so they get set less often (from least to most problematic).
Correct me if I'm wrong, but the difference between (3) and (4) appears to be that under (4), :last-child would still be broken.
:last-child would set (3).  If we didn't make the distinction, there would only be (3) and not (4).
Duplicate of this bug: 414043
Fix by checkin of bug 401291 to trunk, 2008-02-18 22:17 -0800.

I included tests for this bug in that landing.

Unfortunately when I mentioned this bug in the checkin comment, I cited the wrong bug number (bug 75386) in the checkin comment.
Closed: 13 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: Future → mozilla1.9beta4
Duplicate of this bug: 357811
You need to log in before you can comment on or make changes to this bug.