Closed Bug 723102 Opened 12 years ago Closed 12 years ago

[New Tab Page] Can't Hide/Show New Tab Page when closing left tab

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 13
Tracking Status
firefox12 --- fixed

People

(Reporter: virgil.dicu, Assigned: ttaubert)

References

Details

(Whiteboard: [qa!])

Attachments

(1 file)

Mozilla/5.0 (Windows NT 5.1; rv:13.0a1) Gecko/20120201 Firefox/13.0a1

1. Open Firefox Nightly with clean profile.
2. Enable New tab feature (browser.newtab.url--->about:newtab)
3. Restart.
4. Open two new tabs.
5. Close the left one while the second one is focused.
6. Select the Hide/Show New tab Page button in the remaining tab.

Actual result: No action is performed when the button is selected.
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Attached patch patch v1Splinter Review
Simple patch. I accidentally the definition of 'self'. So this was undefined and didn't properly unregister the page.
Attachment #594211 - Flags: review?(jwein)
Comment on attachment 594211 [details] [diff] [review]
patch v1

>+    function unload() gAllPages.unregister(this);

unload doesn't want to return anything, so:

function unload() { gAllPages.unregister(this); }
Comment on attachment 594211 [details] [diff] [review]
patch v1

Review of attachment 594211 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Make the change that Dao recommended and then r=me. Thanks for adding a test :)
Attachment #594211 - Flags: review?(jwein) → review+
Blocks: 723832
(In reply to Dão Gottwald [:dao] from comment #2)
> unload doesn't want to return anything, so:
> 
> function unload() { gAllPages.unregister(this); }

Fixed.

https://hg.mozilla.org/integration/fx-team/rev/f2900ceb8202
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 13
https://hg.mozilla.org/mozilla-central/rev/f2900ceb8202
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Comment on attachment 594211 [details] [diff] [review]
patch v1

[Approval Request Comment]
User impact if declined: Close new tab pages will be held alive.
Risk to taking this patch (and alternatives if risky): Very low.
String changes made by this patch: None.

Needed for New Tab Page testing. Fixes whole page leaks.
Attachment #594211 - Flags: approval-mozilla-aurora?
Alex, this sounds like a pretty key bug we also want to take on Aurora by pref'ing on New Tab per bug 716538.
Comment on attachment 594211 [details] [diff] [review]
patch v1

[Triage Comment]
Low risk, in support of testing for bug 716538. Approving for Aurora 12.
Attachment #594211 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:12.0a2) Gecko/20120220 Firefox/12.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:12.0a2) Gecko/20120221 Firefox/12.0a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a2) Gecko/20120220 Firefox/12.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:13.0a1) Gecko/13.0a1 Firefox/13.0a1
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:13.0) Gecko/13.0 Firefox/13.0a1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/13.0 Firefox/13.0a1

Verified on today's Aurora and today's Nightly with steps in comment 0 on Ubuntu 11.10, Mac OS 10.7 and Windows 7.
Status: RESOLVED → VERIFIED
Whiteboard: [qa!]
You need to log in before you can comment on or make changes to this bug.