594176 - "move to group" should not be an option on the right-click menu for app tabs

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Ian Gilman [:iangilman]]

App tabs are available in all groups, so there's no way to move them into a group.

Comment 1

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 2

[Raymond Lee [:raymondlee]]

Passed try

Comment 3

[Ian Gilman [:iangilman]]

Comment on attachment 476195 [details] [diff] [review]
v1

+  popup(origTab);
+  ok(!document.getElementById("context_tabViewMenu").hidden, 
+     "The tab view menu is hidden for normal tab");

This should say "is not hidden" rather than "is hidden".

f+ with that

Comment 4

[Raymond Lee [:raymondlee]]

Attachment: v1

f+=ian

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

According to bug 587990 comment 66, this fix also repairs a case of tab orphaning, so we should make this bug block.

Comment 6

[Dietrich Ayala (:dietrich)]

Comment on attachment 476473 [details] [diff] [review]
v1

Unless it's changed, the approach we've taken for a long time in scenarios like this is to disable the menu item instead of hiding it. This 1) is less confusing that UI bits just disappearing and 2) gives a visual cue that might help the user intuit why an option is not available.

Comment 7

[Raymond Lee [:raymondlee]]

Attachment: v1

Changed from hidden to disabled for that menu item for app tabs.

Comment 8

[Ian Gilman [:iangilman]]

Comment on attachment 477088 [details] [diff] [review]
v1

+  ok(!document.getElementById("context_tabViewMenu").disabled, 
+     "The tab view menu is visible for normal tab");
+
+  popup(pinnedTab);
+  ok(document.getElementById("context_tabViewMenu").disabled, 
+     "The tab view menu is hidden for pinned tab");

These strings should say disabled/enabled rather than hidden/visible. 

Otherwise looks good.

Comment 9

[Raymond Lee [:raymondlee]]

Attachment: v1

f+=ian

Comment 10

[Dietrich Ayala (:dietrich)]

Comment on attachment 477355 [details] [diff] [review]
v1

blockers don't need approval.

Comment 11

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 12

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 13

[Ian Gilman [:iangilman]]

Pushed to panorama-central: 

http://hg.mozilla.org/users/ian_iangilman.com/panorama-central/rev/84d68aaf1485

I'm removing the "checkin-needed" keyword and adding "[on-panorama-central]" in the whiteboard; the plan is to merge panorama-central directly to mozilla-central once the tree opens again.

Comment 14

[Ian Gilman [:iangilman]]

Landed on mozilla-central:

http://hg.mozilla.org/mozilla-central/rev/84d68aaf1485

Comment 15

[Tracy Walker [:tracy]]

verified with recent nightly build of minefield

Comment 16

[Marcia Knous [:marcia]]

https://litmus.mozilla.org/show_test.cgi?id=13740 added to Litmus.