Last Comment Bug 135141 - [RFE] implement CSS 3 indirect adjacent combinator
: [RFE] implement CSS 3 indirect adjacent combinator
Status: VERIFIED FIXED
: css3
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: Daniel Glazman (:glazou)
: Hixie (not reading bugmail)
Mentors:
Depends on: 420814
Blocks: 65133
  Show dependency treegraph
 
Reported: 2002-04-03 06:54 PST by Daniel Glazman (:glazou)
Modified: 2010-03-05 09:32 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case (not dynamic) (907 bytes, text/html)
2002-04-03 06:58 PST, Daniel Glazman (:glazou)
no flags Details
test case (dynamic) (610 bytes, text/html)
2002-04-03 07:00 PST, Daniel Glazman (:glazou)
no flags Details
patch 1.0, partial fix, not handling dynamic case (2.35 KB, patch)
2002-04-03 07:07 PST, Daniel Glazman (:glazou)
dbaron: review+
dbaron: superreview+
Details | Diff | Splinter Review
patch that doesn't break 80th column (3.83 KB, patch)
2004-01-13 20:00 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review
patch with comments (4.13 KB, patch)
2004-01-13 20:04 PST, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
no flags Details | Diff | Splinter Review

Description Daniel Glazman (:glazou) 2002-04-03 06:54:48 PST
CSS 3 introduces a new combinator '~' for indirect adjacent sibling matching.
For instance :

   foo ~ bar

should match all bar elements following, not necessarily immediately, a foo
element in the list of their parent's children.
We should implement it.
Comment 1 Daniel Glazman (:glazou) 2002-04-03 06:58:33 PST
Created attachment 77447 [details]
test case (not dynamic)
Comment 2 Daniel Glazman (:glazou) 2002-04-03 07:00:45 PST
Created attachment 77448 [details]
test case (dynamic)
Comment 3 Daniel Glazman (:glazou) 2002-04-03 07:07:43 PST
Created attachment 77449 [details] [diff] [review]
patch 1.0, partial fix, not handling dynamic case
Comment 4 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2002-04-03 07:30:53 PST
Can you convince me that there aren't any cases where this causes greedy
matching rather than nondeterministic matching?  (I haven't thought it through
yet.  Perhaps it's not that hard to demonstrate.)
Comment 5 Bradley Baetz (:bbaetz) 2002-07-14 02:48:54 PDT
Heh. I just implemented this myself (before searching for the bug). My patch is
the same as glazou's except for whitespace issues, and I moved the
NS_IS_GREEDY_OPERATOR defintion into nsCSSStyleSheet.cpp, which is the only
place which actually uses that macro.

The dynamic case for '+' is bug 110972, and I imagine that the same fix would
fix '~', too.

dbaron: re comment #4, code later in the function (which you added for bug
24031) tests against NS_IS_GREEDY_OPERATOR, which is modified in this patch to
match '~' as well. The comment currently refers to the descendent combinator,
but it does use the macro to test (I've updated the comment in my tree). IOW, if
we DTRT for descendent selectors, I think that the patch makes us DTRT for ~.
Comment 6 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2002-07-14 15:10:51 PDT
It's clear to me that the macro would no longer be appropriate if someone came
up with a generic non-adjacent first cousin combinator.  I think it will still
work with the non-adjacent sibling combinator (indirect adjacent is a horrible
name -- the point is that the need *not* be adjacent).  I think it should still
be correct in this case, but only because I can't think of a way to break it. 
Perhaps I'll try to write a more formal proof later and see if I can...
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-13 19:57:42 PST
Comment on attachment 77449 [details] [diff] [review]
patch 1.0, partial fix, not handling dynamic case

r+sr=dbaron if you fix the 80th-column violations (i.e., remove the space
inside the parentheses in the definition of NS_IS_GREEDY_OPERATOR and break
lines in nsCSSStyleSheet.cpp.)
Comment 8 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-13 19:58:49 PST
(Note that I just fixed the dynamic issues in my patch for bug 15608.)
Comment 9 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-13 20:00:50 PST
Created attachment 139000 [details] [diff] [review]
patch that doesn't break 80th column

(Since I wrote the patch myself, as bbaetz did, before finding this bug, I'll
attach my version, which doesn't go past the 80th column.)
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-13 20:02:48 PST
Although, actually, it's worth fixing the "there are three combinators" comment
in nsICSSStyleRule.h.
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-13 20:04:46 PST
Created attachment 139001 [details] [diff] [review]
patch with comments
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-14 12:12:46 PST
Fix checked in to trunk, 2004-01-14 12:12 -0800.

I also took bbaetz's suggestion in comment 5 of moving the macro to the .cpp file.
Comment 13 Anne (:annevk) 2004-01-18 07:05:38 PST
-> VERIFIED

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a) Gecko/20040116
Comment 14 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2008-03-03 22:49:23 PST
(In reply to comment #5)
> IOW, if
> we DTRT for descendent selectors, I think that the patch makes us DTRT for ~.

(In reply to comment #6)
> I think it should still
> be correct in this case, but only because I can't think of a way to break it. 

We were wrong.  See bug 420814.


Note You need to log in before you can comment on or make changes to this bug.