Closed
Bug 163556
Opened 22 years ago
Closed 21 years ago
AttributeAffectsStyle should move from nsIStyleSheet to nsIStyleRuleProcessor
Categories
(Core :: CSS Parsing and Computation, defect, P3)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla1.4alpha
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Whiteboard: [patch][whitebox])
Attachments
(2 files, 3 obsolete files)
1.22 KB,
text/html
|
Details | |
81.13 KB,
patch
|
Details | Diff | Splinter Review |
AttributeAffectsStyle belongs next to HasStateDependentStyle on nsIStyleRuleProcessor rather than on nsIStyleSheet. While doing this, the nsCSSStyleSheet implementation could be simplified and converted from nsHashtable to pldhash.
Assignee | ||
Updated•22 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Future
Assignee | ||
Comment 1•22 years ago
|
||
This is work in progress. It compiles, but I haven't tested it yet.
Assignee | ||
Updated•22 years ago
|
Whiteboard: [patch]
Target Milestone: Future → mozilla1.4alpha
Assignee | ||
Comment 2•22 years ago
|
||
I used this testcase and a |printf| right after the change in nsCSSFrameConstructor.cpp to verify that the changes are doing what I expected. (I got |affects==0| for the first and fourth buttons and |affects==1| for the second and third.) Everything seems to work just fine.
Assignee | ||
Updated•22 years ago
|
Attachment #112774 -
Flags: superreview?(bzbarsky)
Attachment #112774 -
Flags: review?(bzbarsky)
Assignee | ||
Updated•22 years ago
|
Attachment #112774 -
Attachment description: patch (work in progress) → patch
Assignee | ||
Comment 3•21 years ago
|
||
Fix leak caused by omitted PL_DHashTableFinish.
Attachment #112774 -
Attachment is obsolete: true
Assignee | ||
Comment 4•21 years ago
|
||
How about a clearEntry callback to fix the array leak, too? :-)
Assignee | ||
Updated•21 years ago
|
Attachment #113701 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #112774 -
Flags: superreview?(bzbarsky)
Attachment #112774 -
Flags: review?(bzbarsky)
Comment 5•21 years ago
|
||
Comment on attachment 113706 [details] [diff] [review] patch >+ AttributeRuleProcessorData(nsIPresContext* aPresContext, >+ nsIContent* aContent, >+ nsIAtom* aAttribute, >+ PRInt32 aModType) Odd indentation.... >+ nsIAtom* medium = nsnull; >+ aPresContext->GetMedium(&medium); >+ AttributeData data(aPresContext, medium, aContent, aAttribute, aModType); >+ WalkRuleProcessors(SheetHasAttributeStyle, &data); >+ NS_IF_RELEASE(medium); I know you just copied that refcounting, but could we make that an nsCOMPtr? And maybe even fix the place you copied from? (we should really have a decent way to not have all this identical-looking code... most of this file is screaming "Macro!" at me... but that's a battle for another day). Nice simplification of the attr hash stuff....
Attachment #113706 -
Flags: superreview+
Attachment #113706 -
Flags: review+
Assignee | ||
Comment 7•21 years ago
|
||
Addresses bz's nsCOMPtr comment.
Attachment #113706 -
Attachment is obsolete: true
Assignee | ||
Comment 8•21 years ago
|
||
Fix checked in to trunk, 2003-02-22 08:10 PST.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Updated•21 years ago
|
Whiteboard: [patch] → [patch][whitebox]
You need to log in
before you can comment on or make changes to this bug.
Description
•