Closed
Bug 147887
Opened 22 years ago
Closed 17 years ago
put different types of pseudos in different atom lists
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
Attachments
(1 file, 3 obsolete files)
770 bytes,
text/html
|
Details |
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.
Assignee | ||
Updated•22 years ago
|
Assignee | ||
Comment 1•22 years ago
|
||
Daniel had some similar work in bug 62843. I also had this partly done in one of my spare trees...
Assignee | ||
Comment 2•22 years ago
|
||
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Future
Assignee | ||
Comment 3•22 years ago
|
||
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.
Comment 4•22 years ago
|
||
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?
Oh yeah. r/sr=roc+moz
Comment 6•22 years ago
|
||
sr=bzbarsky as well.
Assignee | ||
Comment 7•22 years ago
|
||
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 8•22 years ago
|
||
Comment on attachment 106535 [details] [diff] [review] patch > + * 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+
Assignee | ||
Comment 9•22 years ago
|
||
|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...
Comment 10•22 years ago
|
||
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. ;)
Assignee | ||
Comment 11•22 years ago
|
||
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
Assignee | ||
Comment 12•22 years ago
|
||
Comment on attachment 106537 [details] [diff] [review] patch 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.
Comment 13•22 years ago
|
||
: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.
Assignee | ||
Comment 14•22 years ago
|
||
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.
Assignee | ||
Comment 15•22 years ago
|
||
Comment on attachment 106537 [details] [diff] [review] patch 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 functions.
Attachment #106537 -
Attachment is obsolete: true
Comment 16•22 years ago
|
||
first-letter and first-line are confused....
Comment 17•22 years ago
|
||
I suspect these changes in nsCSSFrameConstructor::GetFirstLineStyle: - nsHTMLAtoms::firstLinePseudo, + nsCSSPseudoElements::firstLetter, and in nsCSSFrameConstructor::HaveFirstLineStyle: - nsHTMLAtoms::firstLinePseudo); + nsCSSPseudoElements::firstLetter); ;)
Assignee | ||
Comment 18•22 years ago
|
||
Oops. Thanks for catching that. I just checked in the fix.
Comment 19•21 years ago
|
||
What's left to do here?
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•