Closed Bug 607496 Opened 14 years ago Closed 14 years ago

feed button code cleanup

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 4.0b8

People

(Reporter: Gavin, Assigned: Gavin)

References

Details

Attachments

(1 file)

- 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.
Attached patch patchSplinter Review
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Attachment #486219 - Flags: review?(dolske)
Attachment #486219 - Flags: feedback?(margaret.leibovic)
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 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+
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.

Attachment

General

Created:
Updated:
Size: