Closed Bug 147887 Opened 18 years ago Closed 13 years ago

put different types of pseudos in different atom lists


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






(Reporter: dbaron, Assigned: dbaron)


(Blocks 1 open bug)



(1 file, 3 obsolete files)

I'd like to put different types of pseudos (elements, classes, and our special
globals that we use as pseudo-elements with null content) into three different
lists, with an additional static member function relative to the current atom
lists that tests whether something is an atom in one of the lists.  This will
help with:
 * showing what's going on by making the type distinction clearer
 * allowing callers to distinguish between the types, which will help to fix a
number of bugs that I'm going to add as dependencies.
Daniel had some similar work in bug 62843.  I also had this partly done in one
of my spare trees...
Priority: -- → P3
Target Milestone: --- → Future
I think nsCSSAnonBoxPseudos would be better than nsCSSPseudoGlobals, since
they're really pseudos that we use to style anonymous boxes.

Also, we can define classes:

class nsICSSPseudoElement : public nsIAtom {};
class nsICSSPseudoClass : public nsIAtom {};
class nsICSSAnonBoxPseudo : public nsIAtom {};

and have the atom lists composed of these, and use them for parameter types in
functions.  This could be a second pass after the first round of changes.
Could we get a blanket r=/sr= on this idea so David can move forward with it
without having to check back for every minor fixup?
sr=bzbarsky as well.
Attached patch patch (obsolete) — Splinter Review
This should be pretty much complete.  (My tree is still building, though, since
I just did some search-replace on the patch and reapplied.)  I'm not checking
it in tonight (it's late), so if you want to really review the patch, feel
free.  (It's probably good if you look at the changes in content/shared/.  The
rest are just wrote substitution, with occasional reindenting.)
Attachment #98432 - Attachment is obsolete: true
Comment on attachment 106535 [details] [diff] [review]

> + * This file contains the list of all CSS nsIAtoms and their values. 

This part of the comment isn't really true for the new files. ;)  

MPL please, not NPL for the licenses...

Any reason to keep ":not" named "notPseudo" instead of just "not"?

Other than those nits, looks great.
Attachment #106535 - Flags: superreview+
|not| is a reserved word (it's an alternative representation of ! -- see C++
2.11 clause 2 and 2.5).

I'm a little hesitant about changing licenses when splitting files, but I guess
Netscape has agreed to NPL->MPL conversion (is there an official announcement of
that somewhere)?

I'll fix the comments...
Yeah, I was having a sneaking suspicion that it must be reserved.  Good point
about the file splitting; do whichever is less work with the licenses.  ;)
Attached patch patch (obsolete) — Splinter Review
This fixes the licenses and the comments in the list files.  I also put the
lists in a more "logical" order (grouped by function), and found one unused
anon-box-pseudo (:-moz-wrapper-frame, nice and descriptive), and removed the
corresponding rule from html.css
Attachment #106535 - Attachment is obsolete: true
Comment on attachment 106537 [details] [diff] [review]

A few other things to note:
 * I just change IsAnonBoxPseudo to IsAnonBox in my tree.
 * This removes IsPseudoClass from nsCSSParser.cpp and replaces it with
nsCSSPseudoClasses::IsPseudoClass, which also returns true for ":not". 
However, I think this is OK.  I'll test it once my tree finishes building.
:not() cannot contait :not() and has a different specificity (or rather, takes
its specificity from its argument). Those would be two things to look at in
particular when checking that that change doesn't matter.
The only changes I made that affected :not were in the parser, and I ran a
significant portion of the Selectors test suite, so I think I'm ok.
Comment on attachment 106537 [details] [diff] [review]

Fix checked in, 2002-11-17 07:37 PDT.

I still want to use this bug to use the new types for parameters to some
Attachment #106537 - Attachment is obsolete: true
Attached file broken stuff
first-letter and first-line are confused....
I suspect these changes in nsCSSFrameConstructor::GetFirstLineStyle:

-                                 nsHTMLAtoms::firstLinePseudo,
+                                 nsCSSPseudoElements::firstLetter,

and in nsCSSFrameConstructor::HaveFirstLineStyle:

-                        nsHTMLAtoms::firstLinePseudo);
+                        nsCSSPseudoElements::firstLetter);

Oops.  Thanks for catching that.  I just checked in the fix.
Blocks: 177539
No longer blocks: 177539
What's left to do here?
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.