Closed Bug 996632 Opened 10 years ago Closed 10 years ago

'Close tab' option from the tab context menu should be active for the last tab

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox31 --- verified

People

(Reporter: pauly, Assigned: speaker)

References

Details

(Whiteboard: [good first bug][mentor=dao][lang=js] p=0 s=it-31c-30a-29b.3 [qa!])

Attachments

(1 file, 2 obsolete files)

31.0a1 (2014-04-15), Win 7 x64

STR:
1. Have only one tab open
2. Right click on the tab

AR: 'Close tab' option is grayed out
Seems like we'll just need to remove this code:
http://hg.mozilla.org/mozilla-central/annotate/16e9cda44250/browser/base/content/browser.js#l6988

And update this test:
http://mxr.mozilla.org/mozilla-central/source/browser/components/tabview/test/browser_tabview_bug625195.js
Flags: firefox-backlog?
OS: Windows 7 → All
Hardware: x86_64 → All
Whiteboard: [good first bug][mentor=dao][lang=js]
Version: unspecified → Trunk
I would like to work upon this bug. i went into the folder mozilla-central/browser/base/content/   and found the file "browser.js" in line number 6978 I found the function TabContextMenu. should i make the changes there ??
Assignee: nobody → spkr322
(In reply to Praveenkumar from comment #2)
> I would like to work upon this bug. i went into the folder
> mozilla-central/browser/base/content/   and found the file "browser.js" in
> line number 6978 I found the function TabContextMenu. should i make the
> changes there ??

Yes :)
Flags: firefox-backlog? → firefox-backlog+
Attached patch Patch for the change (obsolete) — Splinter Review
I am attaching the patch for bug.
Attachment #8408029 - Flags: review+
Comment on attachment 8408029 [details] [diff] [review]
Patch for the change

(In reply to Praveenkumar from comment #4)
> Created attachment 8408029 [details] [diff] [review]
> Patch for the change

Hello Praveenkumar, I'd recommend reading https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch to learn about generating a proper patch file and about requesting review on a patch. In this case you would want to set the review flag to "?" since you are asking for someone else to review. The reviewer will set it to "+" if they accept the patch. I have done this for you in this case.
Attachment #8408029 - Flags: review+ → review?(dao)
Comment on attachment 8408029 [details] [diff] [review]
Patch for the change

>--- a/browser/components/tabview/test/browser_tabview_bug625195.js	Mon Apr 14 15:42:17 2014 -0400
>+++ b/browser/components/tabview/test/browser_tabview_bug625195.js	Thu Apr 17 12:19:05 2014 +0530
>@@ -30,7 +30,6 @@
>     whenTabViewIsHidden(function() {
>       popup(gBrowser.tabs[0]);
> 
>-      ok(!document.getElementById("context_closeTab").disabled, "The 'Close tab' menu item is enabled");

You can actually keep this line, it should still work. However, you need to update this line at the beginning of this test:

> ok(document.getElementById("context_closeTab").disabled, "The 'Close tab' menu item is disabled");

Can you please attach a new patch that does this and request review again? Thanks.
Attachment #8408029 - Flags: review?(dao)
Attachment #8408029 - Flags: review-
Attachment #8408029 - Flags: feedback+
Attached patch With the changes (obsolete) — Splinter Review
I have made the changes.
Attachment #8408121 - Flags: review?(dao)
Attachment #8408029 - Attachment is obsolete: true
Attachment #8408121 - Attachment is obsolete: true
Attachment #8408121 - Flags: review?(dao)
Comment on attachment 8408141 [details] [diff] [review]
Please discard the previous patch this one is the correct one.

Looks good, thanks!
Attachment #8408141 - Flags: review?(dao) → review+
Keywords: checkin-needed
landed as https://hg.mozilla.org/integration/fx-team/rev/c281d3b22560

Thanks for contributing to Mozilla!
Keywords: checkin-needed
Whiteboard: [good first bug][mentor=dao][lang=js] → [good first bug][mentor=dao][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c281d3b22560
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [good first bug][mentor=dao][lang=js][fixed-in-fx-team] → [good first bug][mentor=dao][lang=js]
Target Milestone: --- → Firefox 31
Whiteboard: [good first bug][mentor=dao][lang=js] → [good first bug][mentor=dao][lang=js] p=0 s=it-31c-30a-29b.3 [qa?]
Hi Liz, can you review this bug to determine if it requires further QA verification [qa+] or not [qa-].
Flags: needinfo?(lhenry)
Cornel, would you mind verifying this along with your tiles related bugs? Let me know if that's a problem or you think it should go to someone else. Thanks!
Flags: needinfo?(lhenry)
QA Contact: cornel.ionce
Whiteboard: [good first bug][mentor=dao][lang=js] p=0 s=it-31c-30a-29b.3 [qa?] → [good first bug][mentor=dao][lang=js] p=0 s=it-31c-30a-29b.3 [qa+]
Assigning to Paul since he is more familiar with the background for this issue.
QA Contact: cornel.ionce → paul.silaghi
'Close tab' is active and works fine for the last tab.
Verified fixed 31.0a1 (2014-04-25) Win 7, Ubuntu 13.04, OS X 10.8.5
Status: RESOLVED → VERIFIED
Whiteboard: [good first bug][mentor=dao][lang=js] p=0 s=it-31c-30a-29b.3 [qa+] → [good first bug][mentor=dao][lang=js] p=0 s=it-31c-30a-29b.3 [qa!]
Question:
Have only one pinned tab, right click/close tab on it, then start FF again -> the pinned tab is still there. Is this expected ?
Flags: needinfo?(dao)
(In reply to Paul Silaghi, QA [:pauly] from comment #16)
> Question:
> Have only one pinned tab, right click/close tab on it, then start FF again
> -> the pinned tab is still there. Is this expected ?

Yes.
Flags: needinfo?(dao)
You need to log in before you can comment on or make changes to this bug.