Closed Bug 500610 Opened 11 years ago Closed 4 years ago

Dummy headers (groupby view) should have context menus disabled

Categories

(Thunderbird :: Folder and Message Lists, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 43.0

People

(Reporter: rkent, Assigned: alta88)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

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?
Attached patch groupedContext.patch (obsolete) — Splinter Review
No contextmenu for grouped by header rows.
Assignee: nobody → alta88
Attachment #8654528 - Flags: review?(rkent)
Blocks: 655653
Attached patch groupedContext.patch (obsolete) — Splinter Review
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)
Attached patch groupedContext.patch (obsolete) — Splinter Review
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 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+
(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.
Attachment #8656722 - Attachment is obsolete: true
Attachment #8656801 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/790425f3fe961eec53c877c6075f58aa7a6e0ffc
Bug 500610 - Dummy headers (groupby view) should have context menus disabled. r=mkmelin
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0
I think this regressed Bug 1203244.
Depends on: 1203244
Duplicate of this bug: 506519
You need to log in before you can comment on or make changes to this bug.