Closed Bug 163556 Opened 18 years ago Closed 17 years ago

AttributeAffectsStyle should move from nsIStyleSheet to nsIStyleRuleProcessor


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






(Reporter: dbaron, Assigned: dbaron)



(Whiteboard: [patch][whitebox])


(2 files, 3 obsolete files)

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.
Priority: -- → P3
Target Milestone: --- → Future
Blocks: 15608
Attached patch patch (obsolete) — Splinter Review
This is work in progress.  It compiles, but I haven't tested it yet.
Whiteboard: [patch]
Target Milestone: Future → mozilla1.4alpha
Attached file testcase
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.
Attachment #112774 - Flags: superreview?(bzbarsky)
Attachment #112774 - Flags: review?(bzbarsky)
Attachment #112774 - Attachment description: patch (work in progress) → patch
Attached patch patch (obsolete) — Splinter Review
Fix leak caused by omitted PL_DHashTableFinish.
Attachment #112774 - Attachment is obsolete: true
Attached patch patch (obsolete) — Splinter Review
How about a clearEntry callback to fix the array leak, too? :-)
Attachment #113701 - Attachment is obsolete: true
Attachment #112774 - Flags: superreview?(bzbarsky)
Attachment #112774 - Flags: review?(bzbarsky)
Comment on attachment 113706 [details] [diff] [review]

>+  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+
This may fix bug 190625 too
Blocks: 190625
Attached patch patchSplinter Review
Addresses bz's nsCOMPtr comment.
Attachment #113706 - Attachment is obsolete: true
Fix checked in to trunk, 2003-02-22 08:10 PST.
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [patch] → [patch][whitebox]
Duplicate of this bug: 68198
You need to log in before you can comment on or make changes to this bug.