Closed Bug 27827 Opened 25 years ago Closed 25 years ago

Right click on form input text fields causes no further key input to be registered.

Categories

(Core :: XUL, defect, P3)

defect

Tracking

()

VERIFIED FIXED

People

(Reporter: jelwell, Assigned: akkzilla)

References

Details

(Whiteboard: [PDT+] have workaround, approved 3/1 for checkin)

Attachments

(2 files)

I'm using winNT build 2000021408. To Reproduce. 1) Right click on one of the text input (or textareas) fields on this bug. 2) Notice the context menu. 3) Left click somewhere on the blank portion of the page to clear the context menu. 4) Left click into any text input field. 5) Try to type. Shortcuts don't work either. Expected behaviour: allow you to type. :)
Severity: normal → critical
hyatt says this is his new key nav stuff. the key listener is not getting removed for some reason and it's eating all the key events.
Assignee: pinkerton → hyatt
Ender is double-dispatching the mouse down events into the DOM. I believe Akkana was fixing this.
Assignee: hyatt → akkana
Yes, I'll be looking at redesigning the ender event listener in M15.
Status: NEW → ASSIGNED
Target Milestone: M15
*** Bug 29582 has been marked as a duplicate of this bug. ***
nominating for beta1...
Keywords: beta1
Is this really a dup? Everyone commenting on bug 29582 says it was a regression, whereas this one has been around for a while and was being used as a marker for "the ender event listener is broken, we should look into redesigning it after beta when it's not so risky." If we need to address a regression in the short term, in a low-risk way, that bug should perhaps be kept separate.
good point...i'll go check with an older build to see if what i saw in bug 29582 is still there. if it isn't, 29582 is a regression which i'll reopen.
after checking with bits from an older build (2000.2.21), methinks bug 29582 is a dup of this one...
Putting on PDT+ for beta1. But will move to PDT- if not fixed by 03/03 or if this regresses.
Whiteboard: [PDT+]w/b minus on 03/03
Okay, I tried some quick fixes. Disabling the ender event listener altogether causes problems -- xbl stops working. Eventually I'd like to get rid of it, but that's too risky for beta1. Commenting out the section where the ender event listener registers as a mouse listener worked much better: now right-mouse doesn't bring up a context menu (which wasn't working right before anyway -- aside from subsequent events not working, the menu also came up far away from the mouse click location), but it doesn't do any harm and you can continue to type, and XBL key bindings still go through, and left and middle clicks and double clicks still work. I have only tested this for a few minutes on linux. I will test further on other platforms, and run with it as dogfood in my tree once the mail blocker gets fixed. Assuming it tests out on all platforms, I think we should go with this for beta1, and not worry about the lack of context menus in textareas (have they ever worked?). But if we care about context menus in text fields, we'll have to do some more work to figure out how to turn those on.
Whiteboard: [PDT+]w/b minus on 03/03 → [PDT+] testing possible fix
unless things have changed radically, by commenting out the mouse listener you will lose the ability to click and drag on a text control that is lazily instantiated. Try this: open mozilla to some URL left click somewhere in the URL without letting go of the left mouse button, drag. this should do the expected drag-select operation. if it doesn't, you're proposed change probably broke that feature. as an intermediate hack, you could disable propagation of *only* right clicks in the listener. ugg. I agree with Akkana that context menus for text areas are not a requirement for beta. If disabling them makes the app run better, that's fine by me.
Thanks, Steve. Turns out click-and-drag select still works, even in a text field that hasn't been webshellified yet. But I'll pay special attention to the newly-instantiated text field case as I test it on the other platforms tomorrow. The listener had already been disabled for middle clicks, several weeks ago, with no ill effects even on not-yet-instantiated text fields.
sounds great. when you're comfortable with your fix, attach a patch and I'll do some testing too.
Attached patch Initial patch (deleted) — Splinter Review
Nope, the quick fix doesn't work on either windows or mac. We're bitten again by the "event flow is totally different on different platforms" problem. So this may require two or even three different fixes. Of course, I can easily disable right-clicks in the ender event listener just like I already disabled middle clicks, since we've agreed that we don't care whether context menus work. I'll try that approach and see if it breaks anything.
Whiteboard: [PDT+] testing possible fix → [PDT+]
This patch (basically, do the same thing for right-click as we already did for middle-click) is looking much better, on all three platforms. Best of all, not only does it not disable popup menus, but it actually fixes them -- popup menus now come up next to the mouse instead of way up at the top of the screen, and they work correctly, yet do not disable further key events (xbl or typing). Will attach patch. Steve, can you review this? I'll continue to dogfood-test on linux.
Attached patch Simpler, better patch (deleted) — Splinter Review
Whiteboard: [PDT+] → [PDT+] Have workaround, testing now
r=buster. I didn't verify that it works yet, but I will soon. but in terms of a standard code review, the code is fine. so, if it works ok on all platforms, check the sucker in.
Here's the status: The current workaround causes a situation where context menus come up the first time on text fields, but not subsequent times, and don't come up at all on text areas. This is, of course, somewhat confusing to the user. There are three options here: 1. Accept the workaround as is, and release note the sometimes-working context menus. 2. Take the workaround, and disable context menus when over text fields/areas, so they never come up. (Not sure how long this would take, probably less than a day.) 3. Fix the underlying event model that's causing the problem. (May take a long time and involve a lot of code changes; the event model is known to be broken in different ways on each platform. Or I might get lucky and find an easy fix.) PDT team please advise -- what level of fixes are we taking now for beta? I will investigate (3) in any case (it's on my to-do list for M15), but need to know whether to check in (1) or look into (2) in the meantime.
OS: Windows NT → All
Hardware: PC → All
Whiteboard: [PDT+] Have workaround, testing now → [PDT+] see comments regarding date estimate
Oh, the current workaround is one line of code (change a "break" to "return NS_OK" plus a comment explaining it.
Whiteboard: [PDT+] see comments regarding date estimate → [PDT+] have workaround, need approval/decision (see comments)
Whiteboard: [PDT+] have workaround, need approval/decision (see comments) → [PDT+] have workaround, approved 3/1 for checkin
The small patch has been checked in. We should probably consider disabling context menus as a separate bug, since it would involve a completely separate code fix. Anyone who thinks that's important enough to be a beta blocker, please file it. I'll go ahead and close this, as bug 28401 covers the intended M15+ ender event rewrite.
Status: ASSIGNED → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
yay, thx akkana! verif on linux, mac and winNT [opt comm 2000.03.03.08].
Status: RESOLVED → VERIFIED
*** Bug 30334 has been marked as a duplicate of this bug. ***
*** Bug 30337 has been marked as a duplicate of this bug. ***
*** Bug 34188 has been marked as a duplicate of this bug. ***
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: bugzilla → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: