The default bug view has changed. See this FAQ.

nsXULPopupListener::HandleEvent intends to focus contextmenu target on platforms where it's shown on mousedown, but not those where it's shown on mouseup; been doing exact opposite since 2001

RESOLVED FIXED in mozilla9

Status

()

Core
XP Toolkit/Widgets: XUL
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: graememcc, Assigned: graememcc)

Tracking

({regression})

Trunk
mozilla9
regression
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Background: Running my patch for bug 46555 through try, browser/base/content/test/test_contextmenu.html on test failed on Windows, passed on Mac (it's disabled on linux). Adjusting the test for the change in behaviour my patch introduced, and I got the same failure on Mac, but now passing on Windows. While debugging, I had reenabled the test on my Linux box, and had noticed when loading the page being tested in the browser, I was unable to focus the empty text field when right-clicking.

Investigating this, I ended up performing some CVS archaeology.

Bug 48838 ensured that on platforms where the context menu is shown on mousedown (i.e. not Windows/OS2) the element is focused first:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xul/content/src/nsXULPopupListener.cpp&rev=1.64&root=/cvsroot&mark=284-290

AIUI XP_PC was considered a synonym for XP_WIN back in the day.

Along comes version 1.81 of the file in 2001, which #ifndef's the focus-firing code to run only on platforms where NS_CONTEXT_MENU_IS_MOUSEUP isn't defined. Unfortunately, it goes ahead and defines it on platforms where the context menu is shown on mousedown. See http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xul/content/src/nsXULPopupListener.cpp&rev=1.81&root=/cvsroot&mark=70-74 and lines 366-370 in the same file.

For test_contextMenu on linux, (and presumably Mac) this meant the test wasn't exactly testing what it thought it was: when constructing the context menu for the input element, nsWindowRoot was dispatching the various IsCommandEnabled calls to the contenteditable element.

On the other hand, the popup code's been that way for 10 and a half years, and no-one else seems to have complained...
(Assignee)

Comment 1

6 years ago
Created attachment 556073 [details] [diff] [review]
Patch v1

Running through try as we speak...
Attachment #556073 - Flags: review?(enndeakin)
(Assignee)

Comment 2

6 years ago
Apart from a WebGL crash I'm claiming wasn't my fault, try was green across the board

Comment 3

6 years ago
How does test_contextmenu.html fail? I tested this and don't see any failure on windows.
(In reply to Neil Deakin from comment #3)
> How does test_contextmenu.html fail? I tested this and don't see any failure
> on windows.

In comment 0, Graeme explains that this happens when the patch to bug 46555 is applied.

Comment 5

6 years ago
Comment on attachment 556073 [details] [diff] [review]
Patch v1

OK, we could try this and see how it goes.

However, when I enable the test on linux, it fails in the same manner with the patches in both bugs applied.
Attachment #556073 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 6

6 years ago
Neil, sorry missed the bugmail from comment 3.

There was actually a bug relating to widget focus with the test I discovered later - bug 682618 - hence the failures you were seeing either way.

This code looked wrong in any case.

http://hg.mozilla.org/integration/mozilla-inbound/rev/d069439624f5
Assignee: nobody → graememcc_firefox
No longer blocks: 46555
Status: NEW → ASSIGNED
Flags: in-testsuite-
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/ab8c0ebe600b
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9

Updated

6 years ago
Blocks: 686024
You need to log in before you can comment on or make changes to this bug.