597983 - Menu separator should be hidden when there is only one item in the context menu

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Bobby Johnson [:rjohnson19]]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre
Build Identifier:               Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre

After bug 596811 is fixed, in the details view of an active full theme, the only menu item left will be About.

This means an extraneous menu separator appears above it, and it should be hidden in this case.

Reproducible: Always

Steps to Reproduce:
1. Right click in the details area of an active theme (not persona)
Actual Results:  
After bug 596811 is resolved, the only menu item showing will be About with a menu separator above it.

Expected Results:  
No menu separator should be shown.

Comment 1

[Bobby Johnson [:rjohnson19]]

Per bug 596811 comment 2, also should give the menuseparator an id so we can hide just it and not any other menuseparators.

Comment 2

[Bobby Johnson [:rjohnson19]]

Attachment: patch

This hides the menu separator by counting the number of menu items with enabled commands.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 477013 [details] [diff] [review]
patch

>+        if (contextMenu.children[i].nodeName == "menuitem" && gViewController.isCommandEnabled(contextMenu.children[i].command)) {

Wrap this line, so it's no longer than 80 characters.

Code looks good, but I think it would benefit from an automated test. It should fit nicely into browser_bug591465.js (like you did with bug 596811). I think you should be able to just set gProvider.addons[x].permissions = 0; to get all but the "About" menuitem hidden.

Comment 4

[Bobby Johnson [:rjohnson19]]

Attachment: patch v2

I made this patch on top of the patch for 596811, and added testing in essentially the same way, adding a new test to get the single menu item case covered.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 479221 [details] [diff] [review]
patch v2

Great - thanks!

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 479221 [details] [diff] [review]
patch v2

Approved to land after b7

Comment 7

[Dave Townsend [:mossop]]

Fixed, thanks for the patch: http://hg.mozilla.org/mozilla-central/rev/4e480cb2362f

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with builds on Windows and OS X like Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101008 Firefox/4.0b8pre