Closed
Bug 147372
Opened 22 years ago
Closed 21 years ago
Use .hidden and .collapsed instead of [GS]etAttribute
Categories
(SeaMonkey :: Composer, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.3alpha
People
(Reporter: neil, Assigned: cmanske)
Details
(Keywords: perf, polish)
Attachments
(1 file, 2 obsolete files)
|
7.83 KB,
patch
|
Details | Diff | Splinter Review |
I did some perf testing and GetAttribute("hidden") == true is 40% slower than
.hidden while removeAttribute("hidden") is 25% slower than .hidden = false also
I spotted that CollapseItem() is the same as .collapsed and isn't used anyway :-)
|
Reporter | |
Comment 1•22 years ago
|
||
|
|
Reporter | |
Comment 2•22 years ago
|
||
Oops wrong owner!
|
||
Comment 3•22 years ago
|
||
Setting the attributes and setting the DOM properties is not at all the same
thing. Please test interaction with persistence and the like...
|
|
Reporter | |
Comment 4•22 years ago
|
||
bz, persistence only applies to setAttribute(X, "false") which isn't used here.
|
|
||
Comment 5•22 years ago
|
||
Actually, no. It applies to any attribute value...
I'm not saying it's an issue here, just saying it's worth checking. If the
attrs in question are not even being persisted there is definitely no problem.
If they are, then we may have to be more careful.
|
|
Reporter | |
Comment 6•22 years ago
|
||
bz: I agree that setAttribute("hidden" or "collapsed", "false") will persist and
cannot be replaced with .hidden or .collapsed but neither applies here.
|
Assignee | |
Updated•22 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.2beta
|
|
Reporter | |
Comment 7•22 years ago
|
||
Comment on attachment 85168 [details] [diff] [review]
Proposed patch
This has probably bitrotted, right?
|
|
Assignee | |
Comment 8•22 years ago
|
||
neil: yes, I'd say so! Update and I'll review asap.
Target Milestone: mozilla1.2beta → mozilla1.3alpha
|
|
Reporter | |
Comment 9•22 years ago
|
||
Attachment #85168 -
Attachment is obsolete: true
|
|
Reporter | |
Comment 10•22 years ago
|
||
|
|
Reporter | |
Comment 11•22 years ago
|
||
Comment on attachment 106654 [details] [diff] [review]
New patch (except context menu)
Unlike HideItem, CollapseItem is never used.
Attachment #106654 -
Flags: superreview?(alecf)
Attachment #106654 -
Flags: review?(cmanske)
|
|
Reporter | |
Updated•22 years ago
|
Attachment #106655 -
Flags: review?(cmanske)
|
||
Comment 12•22 years ago
|
||
Comment on attachment 106654 [details] [diff] [review]
New patch (except context menu)
sr=alecf
Attachment #106654 -
Flags: superreview?(alecf) → superreview+
|
|
Assignee | |
Updated•22 years ago
|
Attachment #106654 -
Flags: review?(cmanske) → review+
|
|
Reporter | |
Comment 13•22 years ago
|
||
Comment on attachment 106654 [details] [diff] [review]
New patch (except context menu)
Obsoleting checked-in patch.
Attachment #106654 -
Attachment is obsolete: true
|
|
Reporter | |
Comment 14•21 years ago
|
||
Attachment 106655 [details] [diff] got checked in as part of bug 213477.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
|
||
Comment 15•21 years ago
|
||
Comment on attachment 106655 [details] [diff] [review]
Clean up context menu code
...and therefore does not need to be reviewed again.
Attachment #106655 -
Flags: review?(cmanske)
|
||
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•