Dummy headers (groupby view) should have context menus disabled

RESOLVED FIXED in Thunderbird 43.0

Status

Thunderbird
Folder and Message Lists
--
minor
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: rkent, Assigned: alta88)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 43.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

8 years ago
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.)
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?
(Assignee)

Comment 2

2 years ago
Created attachment 8654528 [details] [diff] [review]
groupedContext.patch


No contextmenu for grouped by header rows.
Assignee: nobody → alta88
Attachment #8654528 - Flags: review?(rkent)
(Assignee)

Updated

2 years ago
Blocks: 655653
(Assignee)

Comment 3

2 years ago
Created attachment 8654700 [details] [diff] [review]
groupedContext.patch


additionally fix bad loop if Message-Open in Conversation is selected with grouped header row selected.
Attachment #8654528 - Attachment is obsolete: true
Attachment #8654528 - Flags: review?(rkent)
Attachment #8654700 - Flags: review?(rkent)
(Assignee)

Comment 4

2 years ago
Created attachment 8656722 [details] [diff] [review]
groupedContext.patch

magnus, this is a prereq for the bug you just looked at, and rkent is busy.
Attachment #8654700 - Attachment is obsolete: true
Attachment #8654700 - Flags: review?(rkent)
Attachment #8656722 - Flags: review?(mkmelin+mozilla)

Comment 5

2 years ago
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
Attachment #8656722 - Flags: review?(mkmelin+mozilla) → review+
(Assignee)

Comment 6

2 years ago
(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.
(Assignee)

Comment 7

2 years ago
Created attachment 8656801 [details] [diff] [review]
groupedContext.patch
Attachment #8656722 - Attachment is obsolete: true
Attachment #8656801 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 8

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

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0

Comment 9

2 years ago
I think this regressed Bug 1203244.
Depends on: 1203244
(Assignee)

Updated

2 years ago
Duplicate of this bug: 506519
You need to log in before you can comment on or make changes to this bug.