Closed
Bug 395667
Opened 18 years ago
Closed 18 years ago
New style nsSVGInteger
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
People
(Reporter: longsonr, Assigned: longsonr)
Details
Attachments
(1 file, 3 obsolete files)
59.75 KB,
patch
|
Details | Diff | Splinter Review |
Assignee | ||
Updated•18 years ago
|
Assignee: nobody → longsonr
Assignee | ||
Comment 1•18 years ago
|
||
Switch Integers over to the style of nsSVGLength2 and nsSVGNumber2.
Attachment #280357 -
Flags: review?(tor)
Assignee | ||
Updated•18 years ago
|
Attachment #280357 -
Flags: review?(tor)
Assignee | ||
Comment 2•18 years ago
|
||
Use default values properly with invalid dual value attributes.
Attachment #280357 -
Attachment is obsolete: true
Attachment #280363 -
Flags: review?(tor)
Assignee | ||
Comment 4•18 years ago
|
||
Oops
Attachment #280363 -
Attachment is obsolete: true
Attachment #280576 -
Flags: review?(tor)
Attachment #280363 -
Flags: review?(tor)
Attachment #280576 -
Flags: review?(tor) → review+
Assignee | ||
Updated•18 years ago
|
Attachment #280576 -
Flags: superreview?(roc)
Comment on attachment 280576 [details] [diff] [review]
complete patch
We should keep an eye on the overhead of checking for each type of attribute in nsSVGElement. If it becomes noticeable, we could add a virtual GetInfoTypes() method that returns a set of flags indicating which Get*Info methods return non-empty lists.
+ if (aName == *intInfo.mIntegerInfo[i].mName) {
+ intInfo.mIntegers[i].Init(i, intInfo.mIntegerInfo[i].mDefaultValue);
+ DidChangeInteger(i, PR_FALSE);
+ break;
+ }
filterRes is being used twice in an IntegerAttributesInfo, so this 'break' is no good, right?
+ int mDefaultValue;
PRInt32?
+ { &nsGkAtoms::filterRes, 0 },
+ { &nsGkAtoms::filterRes, 0 }
This is a little freaky, but I guess it will work OK with the fix above. But please document somewhere which kinds of AttributeInfos are allowed to have duplicate attributes and which aren't.
// Create mapped properties:
This comment can go
+ mIntegerAttributes[FILTERRES_Y].SetBaseValue(resX, this, PR_FALSE);
resY
Attachment #280576 -
Flags: superreview?(roc)
Attachment #280576 -
Flags: superreview+
Attachment #280576 -
Flags: approval1.9+
Assignee | ||
Comment 6•18 years ago
|
||
(In reply to comment #5)
>
> + int mDefaultValue;
>
> PRInt32?
Done.
>
> + { &nsGkAtoms::filterRes, 0 },
> + { &nsGkAtoms::filterRes, 0 }
>
> This is a little freaky, but I guess it will work OK with the fix above. But
> please document somewhere which kinds of AttributeInfos are allowed to have
> duplicate attributes and which aren't.
Well spotted. Basically *any* types may and actually already do have duplicate attributes. I've just removed the breaks for all types as per your suggestion. Since this applies to everything I've not put in any specific comment.
>
> // Create mapped properties:
>
> This comment can go
It still applies to the stuff below, although there's only one left now.
rv = AddMappedSVGValue(nsGkAtoms::href, mHref, kNameSpaceID_XLink);
>
> + mIntegerAttributes[FILTERRES_Y].SetBaseValue(resX, this, PR_FALSE);
>
> resY
>
I really did mean resX here. This the case that you have filterRes="300" which means set both the X and Y filter resolution to 300. We scan one value (resX) and then use it for both FILTERRES_X and FILTERRES_Y. At this point the value of the resY variable is undefined.
Attachment #280576 -
Attachment is obsolete: true
Assignee | ||
Comment 7•18 years ago
|
||
checked in.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•