AttributeAffectsStyle should move from nsIStyleSheet to nsIStyleRuleProcessor

RESOLVED FIXED in mozilla1.4alpha

Status

()

Core
CSS Parsing and Computation
P3
normal
RESOLVED FIXED
16 years ago
7 years ago

People

(Reporter: dbaron, Assigned: dbaron)

Tracking

Trunk
mozilla1.4alpha
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch][whitebox])

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

16 years ago
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

16 years ago
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → Future
(Assignee)

Updated

16 years ago
Blocks: 15608
(Assignee)

Comment 1

16 years ago
Created attachment 112774 [details] [diff] [review]
patch

This is work in progress.  It compiles, but I haven't tested it yet.
(Assignee)

Updated

16 years ago
Whiteboard: [patch]
Target Milestone: Future → mozilla1.4alpha
(Assignee)

Comment 2

16 years ago
Created attachment 112785 [details]
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.
(Assignee)

Updated

16 years ago
Attachment #112774 - Flags: superreview?(bzbarsky)
Attachment #112774 - Flags: review?(bzbarsky)
(Assignee)

Updated

16 years ago
Attachment #112774 - Attachment description: patch (work in progress) → patch
(Assignee)

Comment 3

16 years ago
Created attachment 113701 [details] [diff] [review]
patch

Fix leak caused by omitted PL_DHashTableFinish.
Attachment #112774 - Attachment is obsolete: true
(Assignee)

Comment 4

16 years ago
Created attachment 113706 [details] [diff] [review]
patch

How about a clearEntry callback to fix the array leak, too? :-)
(Assignee)

Updated

16 years ago
Attachment #113701 - Attachment is obsolete: true
Attachment #112774 - Flags: superreview?(bzbarsky)
Attachment #112774 - Flags: review?(bzbarsky)
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

16 years ago
Created attachment 114205 [details] [diff] [review]
patch

Addresses bz's nsCOMPtr comment.
Attachment #113706 - Attachment is obsolete: true
(Assignee)

Comment 8

16 years ago
Fix checked in to trunk, 2003-02-22 08:10 PST.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Updated

16 years ago
Whiteboard: [patch] → [patch][whitebox]
(Assignee)

Updated

7 years ago
Duplicate of this bug: 68198
You need to log in before you can comment on or make changes to this bug.