If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Right clicking on column header show an irrelevant context menu

RESOLVED FIXED in Thunderbird 3.1b2

Status

Thunderbird
Mail Window Front End
--
minor
RESOLVED FIXED
10 years ago
4 years ago

People

(Reporter: gkw, Assigned: Magnus Melin)

Tracking

(Blocks: 1 bug)

Trunk
Thunderbird 3.1b2
Bug Flags:
wanted-thunderbird3 +

Thunderbird Tracking Flags

(thunderbird3.1 beta2-fixed)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

10 years ago
Created attachment 320225 [details]
screenshot

Occurred on Ubuntu Linux while testing Shredder Alpha 1 build 1, with build ID
version 3.0a1 (2008050714).

Try right clicking on the column header while a message is selected; instead of getting a context menu for the column header, I get a context menu for the message instead. See screenshot.
Flags: wanted-thunderbird3?
(Assignee)

Comment 1

10 years ago
That context menu affects the currently selected email (so not totally irrelevant). What would a context menu for the column header display? I suppose it could just do nothing...
OS: Linux → All
Hardware: PC → All
#thunderbird tells me a common behavior on windows is to show a column picker menu (bug 148545). That would seem to make a fair bit of sense.

Comment 3

8 years ago
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.2pre) Gecko/20090727 Shredder/3.0b4pre ID:20090727034746

is still showing this very menu. i agree with comment #2 and bug 148545.
(shouldn't this be marked as a dupe of bug 148545 then?)
(Assignee)

Comment 4

8 years ago
Well it's two different issue, though they should fix each other, so let's just add a dependency.
Assignee: nobody → mkmelin+mozilla
Blocks: 148545
Flags: wanted-thunderbird3? → wanted-thunderbird3+

Comment 5

8 years ago
so what will happen in regard to this "issue"?

Comment 6

8 years ago
Created attachment 434251 [details]
screenshot of missing context menu on thunderbird vs one on OE

Comment 7

8 years ago
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

Has this behavior been changed? Now, I don't see any menu - just a small square stub (no messages were selected, see screenshot).

With regards to comment #1: Context actions/menus are usually performed on the object under the mouse cursor in the current context, not the selected item. If you select an object/icon on the Desktop and then right-click on a blank space on the Desktop, the context menu would show actions relevant for the Desktop and not the previously selected object/icon.
I'm seeing that if there is a selected message in the list you'll get the message context menu when right clicking on the column header.  If nothing is selected in the list (because it's empty or otherwise) then you'll get a very small box as seen in the comparison screenshot.

It makes sense to me that we could at least offer the sort order options as OE does.  We don't have a column dialog so the 'Columns...' option wouldn't make sense for us.  Perhaps other bugs like bug 148545 can explore additional options to add if it seems necessary, which I'm not sure it is.
(Assignee)

Comment 9

8 years ago
Created attachment 437089 [details] [diff] [review]
proposed fix - wip

This fixes it, but mozmill refuses to right click the header :(
Sid ^^^ ?
hm, we've been using EventUtils.synthesizeMouse directly for this, apparently. <http://mxr.mozilla.org/comm-central/source/mail/test/mozmill/shared-modules/test-folder-display-helpers.js#856> I guess asuth would know.

You might try offsetting the click by a few pixels: <https://developer.mozilla.org/en/Mozmill/Mozmill_Controller_Object#rightClick%28%29>
Right, the right-click event itself is insufficient.  Someone needs to send a contextmenu event like in the link sid gave.

Originally I assumed I was just hacking around stuff there, but from the recent thread in moz.dev.platform titled "Automate testing of a context menu actions" it would appear that it is, in fact, canon.
(Assignee)

Comment 13

8 years ago
Created attachment 439310 [details] [diff] [review]
proposed fix, v2

Thx for the tips! Offsetting the rightClick a few px worked.
Attachment #437089 - Attachment is obsolete: true
Attachment #439310 - Flags: ui-review?(clarkbw)
Attachment #439310 - Flags: review?(sid.bugzilla)
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird 3.1b2
Comment on attachment 439310 [details] [diff] [review]
proposed fix, v2

The context menu showing up on the right edge regardless of where you click feels very wrong, but I'm going to let clarkbw make the call on that.

>   setTarget : function CM_setTarget(aNode) {
...
>+        // The column header was clicked, show the column picker.
>+        let treecols = aNode.parentNode;
>+        let nodeList = document.getAnonymousNodes(treecols);
>+        let treecolpicker;

camelCase please.

>+        let popup = document.getAnonymousElementByAttribute(treecolpicker, "anonid", "popup");
>+        treecolpicker.buildPopup(popup);
>+        popup.showPopup(treecolpicker, -1, -1, "popup", "bottomright", "topright");
>+
>+        this.shouldDisplay = false;

This should be outside the block to retain the semantics for when you're not treecol. Is it possible to come up with a test for this? If so, please add one.

> 
>+// make SOLO_TEST=folder-display/test-right-click-middle-click-messages.js

Please remove this comment.

> /**
>+ * Test that clicking on the column header shows the column picker.
>+ */
>+function test_right_click_column_header_shows_col_picker() {
>+  be_in_folder(folder);
>+
>+  // Right click the subject columen header (must use 10 10 offset to make it work!)
>+  // This should show the column picker popup.
>+  mc.rightClick(mc.eid("subjectCol"), 10, 10);

Is it possible for script blocking to make this asynchronous? Our typical idiom is to spin a loop using waitForEval waiting for the event to happen, which in this case is for the popup to be displayed, and fail if it isn't.

>+
>+  let threadCols = mc.window.document.getElementById("threadCols");
>+  let treecolpicker = mc.window.document.getAnonymousNodes(threadCols).item(1);

- camelCase please.

- It isn't immediately clear what makes item(1) so special. Could you please add a comment explaining this?

>+  let popup = mc.window.document.getAnonymousElementByAttribute(
>+                treecolpicker, "anonid", "popup");
>+  // Check that the popup has been opened (or will be once the request finshes).
>+  if (popup.state != "open" && popup.state != "showing")

Yeah, please spin a loop waiting for the popup to be displayed (until its state is "open") instead of this.

I'd like to have another look at the fix, so minusing.
Attachment #439310 - Flags: review?(sid.bugzilla) → review-
(Assignee)

Comment 15

8 years ago
(In reply to comment #14)
> >+        this.shouldDisplay = false;
> 
> This should be outside the block to retain the semantics for when you're not
> treecol. 

Actually no. This was a bug in the original code, but it didn't matter since shouldDisplay was unused. (Having it inside the block would make us have no thread-pane context menu.)
(Assignee)

Comment 16

8 years ago
Created attachment 439777 [details] [diff] [review]
proposed fix, v3

Addressing review comments
Attachment #439310 - Attachment is obsolete: true
Attachment #439777 - Flags: ui-review?(clarkbw)
Attachment #439777 - Flags: review?(sid.bugzilla)
Attachment #439310 - Flags: ui-review?(clarkbw)
Comment on attachment 439777 [details] [diff] [review]
proposed fix, v3

r+ as long as clarkbw okays the position of the menu. Thanks!
Attachment #439777 - Flags: review?(sid.bugzilla) → review+
Comment on attachment 439777 [details] [diff] [review]
proposed fix, v3

This menu should be showing up under the mouse.  Other than that this looks great, ui-r+ with that change.
Attachment #439777 - Flags: ui-review?(clarkbw) → ui-review-
(Assignee)

Comment 19

8 years ago
Created attachment 439978 [details] [diff] [review]
proposed fix, v4

For checkin. 
Carrying forward r=sid0, and adding ui-r=clarkbw (changed the popup position)
Attachment #439777 - Attachment is obsolete: true
(Assignee)

Comment 20

8 years ago
changeset:   5463:840db1e6bc6e
http://hg.mozilla.org/comm-central/rev/840db1e6bc6e

->FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
status-thunderbird3.1: --- → beta2-fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.