Closed
Bug 607496
Opened 14 years ago
Closed 14 years ago
feed button code cleanup
Categories
(Firefox :: General, defect)
Firefox
General
Tracking
()
RESOLVED
FIXED
Firefox 4.0b8
People
(Reporter: Gavin, Assigned: Gavin)
References
Details
Attachments
(1 file)
|
(deleted),
patch
|
Dolske
:
review+
Margaret
:
feedback+
Dolske
:
approval2.0+
|
Details | Diff | Splinter Review |
- the patch for bug 596731 re-added some code that uncollapses the button, which isn't necessary now that we never collapse it
- the code prior to bug 578967 was confusing, in that the click handler relied on an attribute being set on the button. Maybe because we used the "feed" attribute for styling? Either way, unnecessary now.
|
Assignee | |
Comment 1•14 years ago
|
||
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #486219 -
Flags: review?(dolske)
Attachment #486219 -
Flags: feedback?(margaret.leibovic)
|
||
Comment 2•14 years ago
|
||
Comment on attachment 486219 [details] [diff] [review]
patch
Looks good to me. It's nice to get rid of that attribute if we don't need it.
Attachment #486219 -
Flags: feedback?(margaret.leibovic) → feedback+
|
||
Comment 3•14 years ago
|
||
Comment on attachment 486219 [details] [diff] [review]
patch
>+ if (!haveFeeds) {
> this._feedMenuitem.setAttribute("disabled", "true");
> this._feedMenupopup.setAttribute("hidden", "true");
> this._feedMenuitem.removeAttribute("hidden");
> } else {
While you're cleaning up, you could just make the if-block return, and drop / de-indent the else-block.
But I don't care either way. :)
Attachment #486219 -
Flags: review?(dolske)
Attachment #486219 -
Flags: review+
Attachment #486219 -
Flags: approval2.0+
|
|
Assignee | |
Comment 4•14 years ago
|
||
Landed with that change:
http://hg.mozilla.org/mozilla-central/rev/b2eeec6f14e7
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b8
You need to log in
before you can comment on or make changes to this bug.
Description
•