500610 - Dummy headers (groupby view) should have context menus disabled

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Kent James (:rkent)]

When I right click on a groupby header, the context menus (Reply to, Mark, etc.) are enabled - but fail in various ways if you attempt to run them.

(TB3 beta1 spewed errors to the console in this case - but ultimately did the right thing, leaving the menus disabled.)

Comment 1

[Thomas D. (:thomas8)]

rkent, can you provide STR, Actual Results, and Expected Results with some more detail?

Is this really a regression related to Gloda global search UI (introduced by bug 474701), i.e. does this really block gloda-ui-regressions tracker 497199?

Comment 2

[alta88]

Attachment: groupedContext.patch

No contextmenu for grouped by header rows.

Comment 3

[alta88]

Attachment: groupedContext.patch

additionally fix bad loop if Message-Open in Conversation is selected with grouped header row selected.

Comment 4

[alta88]

Attachment: groupedContext.patch

magnus, this is a prereq for the bug you just looked at, and rkent is busy.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 8656722 [details] [diff] [review]
groupedContext.patch

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

Yeah I had this one applied too while testing the other patch. Seemed to work fine.

r=mkmelin with the below fixed/answered

::: mail/base/content/mail3PaneWindowCommands.js
@@ +306,5 @@
>                 gFolderDisplay.selectedMessage.folder &&
>                 gFolderDisplay.selectedMessage.folder.server.canHaveFilters;
>        case "cmd_openConversation":
>          return gFolderDisplay.selectedCount == 1 &&
> +               gFolderDisplay.selectedMessages.length > 0 &&

isn't this line checking the same as the one above?

::: mail/base/modules/dbViewWrapper.js
@@ +1911,5 @@
>    /**
> +   * @return true if the row at the given index is a grouped view dummy header
> +   *     row, false if anything else.
> +   */
> +  isGroupedByHeaderAtIndex: function (aViewIndex) {

nit: no space after function

Comment 6

[alta88]

(In reply to Magnus Melin from comment #5)
> Comment on attachment 8656722 [details] [diff] [review]
> groupedContext.patch
> 
> Review of attachment 8656722 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yeah I had this one applied too while testing the other patch. Seemed to
> work fine.
> 
> r=mkmelin with the below fixed/answered
> 
> ::: mail/base/content/mail3PaneWindowCommands.js
> @@ +306,5 @@
> >                 gFolderDisplay.selectedMessage.folder &&
> >                 gFolderDisplay.selectedMessage.folder.server.canHaveFilters;
> >        case "cmd_openConversation":
> >          return gFolderDisplay.selectedCount == 1 &&
> > +               gFolderDisplay.selectedMessages.length > 0 &&
> 
> isn't this line checking the same as the one above?
> 

no, the first is a treeView selected row count and the second is an array of messages, of which a dummy row is excluded.  so selectedMessages.length == 1 is what was meant; patch will be updated with just that.

Comment 7

[alta88]

Attachment: groupedContext.patch

Comment 8

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/790425f3fe961eec53c877c6075f58aa7a6e0ffc
Bug 500610 - Dummy headers (groupby view) should have context menus disabled. r=mkmelin

Comment 9

[Stefan Sitter [:ssitter]]

I think this regressed Bug 1203244.