Last Comment Bug 500610 - Dummy headers (groupby view) should have context menus disabled
: Dummy headers (groupby view) should have context menus disabled
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: Folder and Message Lists (show other bugs)
: Trunk
: All All
-- minor (vote)
: Thunderbird 43.0
Assigned To: alta88
:
:
Mentors:
: 506519 (view as bug list)
Depends on: 1203244
Blocks: gloda-ui-regressions 655653
  Show dependency treegraph
 
Reported: 2009-06-26 01:12 PDT by Kent James (:rkent)
Modified: 2015-11-13 13:30 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
groupedContext.patch (2.37 KB, patch)
2015-08-29 10:05 PDT, alta88
no flags Details | Diff | Splinter Review
groupedContext.patch (4.04 KB, patch)
2015-08-30 20:26 PDT, alta88
no flags Details | Diff | Splinter Review
groupedContext.patch (4.03 KB, patch)
2015-09-03 11:38 PDT, alta88
mkmelin+mozilla: review+
Details | Diff | Splinter Review
groupedContext.patch (4.06 KB, patch)
2015-09-03 13:51 PDT, alta88
alta88: review+
Details | Diff | Splinter Review

Description User image Kent James (:rkent) 2009-06-26 01:12:33 PDT
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 User image Thomas D. (currently busy elsewhere; needinfo?me) 2012-01-11 05:40:59 PST
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 User image alta88 2015-08-29 10:05:42 PDT
Created attachment 8654528 [details] [diff] [review]
groupedContext.patch


No contextmenu for grouped by header rows.
Comment 3 User image alta88 2015-08-30 20:26:03 PDT
Created attachment 8654700 [details] [diff] [review]
groupedContext.patch


additionally fix bad loop if Message-Open in Conversation is selected with grouped header row selected.
Comment 4 User image alta88 2015-09-03 11:38:59 PDT
Created attachment 8656722 [details] [diff] [review]
groupedContext.patch

magnus, this is a prereq for the bug you just looked at, and rkent is busy.
Comment 5 User image Magnus Melin 2015-09-03 12:54:04 PDT
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 User image alta88 2015-09-03 13:49:57 PDT
(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 User image alta88 2015-09-03 13:51:19 PDT
Created attachment 8656801 [details] [diff] [review]
groupedContext.patch
Comment 8 User image aleth [:aleth] 2015-09-05 11:23:21 PDT
https://hg.mozilla.org/comm-central/rev/790425f3fe961eec53c877c6075f58aa7a6e0ffc
Bug 500610 - Dummy headers (groupby view) should have context menus disabled. r=mkmelin
Comment 9 User image Stefan Sitter 2015-09-09 11:43:55 PDT
I think this regressed Bug 1203244.
Comment 10 User image alta88 2015-11-13 13:30:22 PST
*** Bug 506519 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.