DeCOMtaminate nsICSSPseudoComparator method signature

RESOLVED FIXED in mozilla2.0b3

Status

()

defect
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: craig.topper, Assigned: craig.topper)

Tracking

(Blocks 1 bug)

Trunk
mozilla2.0b3
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

9 years ago
Posted patch DeCOM PseudoMatches (obsolete) — Splinter Review
Change PseudoMatches to return PRBool instead of using an outparam.

Also I don't think this class needs to support QueryFrame. There are no places that Query for it.
Attachment #455892 - Flags: review?(dbaron)
Assignee

Comment 1

9 years ago
Attachment #455893 - Flags: review?(dbaron)

Comment 2

9 years ago
Comment on attachment 455892 [details] [diff] [review]
DeCOM PseudoMatches

>diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp
>-        PRBool matches = PR_TRUE;
>-        aData->mComparator->PseudoMatches(aData->mPseudoTag, value->mSelector,
>-                                          &matches);
>-        if (matches) {
>+        if (aData->mComparator->PseudoMatches(aData->mPseudoTag, value->mSelector,
>+                                              &matches)) {

Missed a spot; still has &matches.
Assignee

Comment 3

9 years ago
> Missed a spot; still has &matches.

Thanks. My original patch failed to apply after I updated my tree and I guess I
was a bit careless with redoing it.
Assignee

Comment 4

9 years ago
Attachment #455892 - Attachment is obsolete: true
Attachment #455895 - Flags: review?(dbaron)
Attachment #455892 - Flags: review?(dbaron)
Assignee

Updated

9 years ago
Attachment #455895 - Flags: review?(dbaron) → review?(bzbarsky)
Assignee

Updated

9 years ago
Attachment #455893 - Flags: review?(dbaron) → review?(bzbarsky)
Comment on attachment 455893 [details] [diff] [review]
Remove QueryFrame support

r=bzbarsky
Attachment #455893 - Flags: review?(bzbarsky) → review+
Comment on attachment 455895 [details] [diff] [review]
Fixed to remove extra argument from caller of PseudoMatches

r=bzbarsky, but I really wonder whether we'll ever have a second impl of this, and if not whether we should just push mScratchArray up to nsICSSPseudoComparator (with a different name, I would think) and inline it or something.

Oh, and aTag is unused, right?  Want to remove it from the signature while you're here?
Attachment #455895 - Flags: review?(bzbarsky) → review+
Assignee

Comment 7

9 years ago
Attachment #458251 - Flags: review?(bzbarsky)
Comment on attachment 458251 [details] [diff] [review]
Remove aTag argument as bz suggested

r=bzbarsky
Attachment #458251 - Flags: review?(bzbarsky) → review+
Assignee

Updated

9 years ago
Keywords: checkin-needed
This needs approval2.0 before it can land.
Keywords: checkin-needed
Assignee

Updated

9 years ago
Attachment #455893 - Flags: approval2.0?
Assignee

Updated

9 years ago
Attachment #455895 - Flags: approval2.0?
Assignee

Updated

9 years ago
Attachment #458251 - Flags: approval2.0?
Need risk/reward assessment here: does this improve pageload times, and how risky is it?
Probably not (this isn't used by pages much) and very safe in that order.
Comment on attachment 455893 [details] [diff] [review]
Remove QueryFrame support

a=me for all of them
Attachment #455893 - Flags: approval2.0? → approval2.0+

Updated

9 years ago
Attachment #455895 - Flags: approval2.0?

Updated

9 years ago
Attachment #458251 - Flags: approval2.0?
You need to log in before you can comment on or make changes to this bug.