Closed Bug 341683 Opened 14 years ago Closed 12 years ago

second argument to nsAttrValue::ParseSpecialIntValue unused

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9beta4

People

(Reporter: dbaron, Assigned: mats)

Details

Attachments

(1 file, 1 obsolete file)

Now that bug 333352, the second argument to nsAttrValue::ParseSpecialIntValue is unused (all callers pass PR_FALSE), so it should be removed.
Attached patch Patch rev. 1 (obsolete) — Splinter Review
Assignee: dbaron → mats.palmgren
Status: NEW → ASSIGNED
Attachment #301005 - Flags: superreview?(dbaron)
Attachment #301005 - Flags: review?(dbaron)
Comment on attachment 301005 [details] [diff] [review]
Patch rev. 1

r+sr=dbaron, although you should probably at least run this by a content peer (maybe sicking)
Attachment #301005 - Flags: superreview?(dbaron)
Attachment #301005 - Flags: superreview+
Attachment #301005 - Flags: review?(dbaron)
Attachment #301005 - Flags: review+
Comment on attachment 301005 [details] [diff] [review]
Patch rev. 1

Jonas, this is just removing code that's not used anymore, any objections?
Attachment #301005 - Flags: review?(jonas)
Comment on attachment 301005 [details] [diff] [review]
Patch rev. 1

>   enum ValueType {
>     eString =       0x00, //    00
>                           //    01  this value indicates an 'misc' struct
>     eAtom =         0x02, //    10
>     eInteger =      0x03, // 00011
>     eColor =        0x07, // 00111
>-    eProportional = 0x0B, // 01011
>     eEnum =         0x0F, // 01111  This should eventually die
>     ePercent =      0x13, // 10011
>     // Values below here won't matter, they'll be stored in the 'misc' struct
>     // anyway
>     eCSSStyleRule = 0x14,
>     eAtomArray =    0x15 
> #ifdef MOZ_SVG
>     ,eSVGValue =    0x16
> #endif
>   };

So we really should move up the other enum values here. I.e. make eEnum be 0x0B and ePercent 0x0F and so on. Which means that we'd only need 4 bits to store the type for integers.

All you should need to do is to change the NS_ATTRVALUE_INTEGERTYPE_BITS define to 4

Also fix the comments to only show 4 bits.

r=me with that.
Attachment #301005 - Flags: review?(jonas) → review+
Good catch, thanks.
Attached patch Patch rev. 2Splinter Review
With Jonas' nit fixed.
Attachment #301005 - Attachment is obsolete: true
Attachment #301125 - Flags: approval1.9?
Attachment #301125 - Flags: approval1.9? → approval1.9+
mozilla/content/base/src/nsAttrValue.cpp 	3.29
mozilla/content/base/src/nsAttrValue.h 	3.22
mozilla/content/html/content/src/nsGenericHTMLElement.cpp 	1.752
mozilla/content/html/content/src/nsHTMLDivElement.cpp 	1.71
mozilla/content/html/content/src/nsHTMLFrameElement.cpp 	1.71
mozilla/content/html/content/src/nsHTMLHRElement.cpp 	1.84
mozilla/content/html/content/src/nsHTMLIFrameElement.cpp 	1.108
mozilla/content/html/content/src/nsHTMLInputElement.cpp 	1.474
mozilla/content/html/content/src/nsHTMLSharedElement.cpp 	1.69
mozilla/content/html/content/src/nsHTMLTableCellElement.cpp 	1.112
mozilla/content/html/content/src/nsHTMLTableColElement.cpp 	1.88
mozilla/content/html/content/src/nsHTMLTableElement.cpp 	1.169
mozilla/content/html/content/src/nsHTMLTableRowElement.cpp 	1.122
mozilla/content/html/content/src/nsHTMLTableSectionElement.cpp 	1.92 

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.