209033 - (Shockwave, Flash, ?) all typed letters (from kbd) appear TWICE in dialog windows

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[who cares]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4) Gecko/20030529
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4) Gecko/20030529

Once connected to a given "play room", any text that is typed from the keyboard
is wrongly echoed twice on the screen.
For instance, typed text "hello" is displayed (and recorded) as "hheellllo".

Reproducible: Always

Steps to Reproduce:
1. connect to http://www.isketch.net/isketch.shtml
2. define arbitrary username (i.e. toto356)
3. check that this username already appears as "ttoottoo335566"
4. etc.

Actual Results:  
Same problem happens for any given typed text.

Expected Results:  
Only echoes once each single letter.

none

Comment 1

[Jeff D. Hanson]

WFM:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030609

Comment 2

[WD]

209039

Comment 3

[Chris Casciano]

Some points of testing:


Mozilla 1.3.1 OS X the plug-in area doesn't even draw... just collapses
Mozilla Trunk Build 2003061103 OS X I'm having this problem
Camino 2003060104.. This bug WFM  (although the text field is oddly black)
Safari v74 WFM just fine (so I doubt its the bugins fault totally)

[Macromedia Shockwave for Director Netscape plug-in, version 8.5.1]

Comment 4

[Chris Casciano]

this also is seen in yesterdays 1.4 branch build... gonna chase down a few older
builds to see if this is a regression.

Comment 5

[Chris Casciano]

You can see this bug in the login form on that page as well. I'm Also seeing
this on other plugin based  media not limited to shockwave. For example the
search dialog on the macromedia web site suffers this same problem & that is
Flash based (latest plugin - 6.0 r79).

http://www.macromedia.com/

WFM:
 2003050714 OS X (1.4b)
 2003051608 OS X (Trunk)
 2003052307 OS X (Trunk)

Broke in:
 2003052905 OS X (Brach 1.4RC1)
 2003060103 OS X (Trunk)

I also did not see this on the Win2k side (2003061104 Trunk)

Setting blocking 1.4? flag, I realize this is very late in the game. I also
realize there's not much here to go on and no patch in sight. but this is a
fiercly UGLY late in the game regression that would horrible to get out in a
release.

Comment 6

[Chris Casciano]

*** Bug 209039 has been marked as a duplicate of this bug. ***

Comment 7

[Chris Casciano]

Greg, happen to have any builds in the regression window (052307-052905)?

Comment 8

[Peter Lubczynski]

ooo, this is really bad, nominating. Plugins are almost completely unusable
because typing causes two key events. Internal testcase:
http://slip.mcom.com/shrir/flashedit.html

Using bonsai, I've tracked this down to the checkin by bryner to
nsMacEventHandler.cpp to fix bug 206917. If I backout that fix, this bug does
not happen.

Comment 9

[Brian Ryner (not reading)]

Well, we're now dispatching both keydown and keypress events on mac (keydown
events were broken for a long time).  Looking at
nsObjectFrame::KeyDown/KeyPress, the comments seem to suggest that we pass
KeyPress events to the plugin only on mac.  Are you sure this particular plugin
isn't typing keys incorrectly for keydown, when it should do it on keypress?  (I
don't know much about the history of why we only send keypress to plugins on mac)

Comment 10

[Chris Casciano]

A little additional testing (2003061608 trunk os x):

I'm *not* seeing this problem with JavaApplets as tested on:

http://javaboutique.internet.com/calculator/
http://javaboutique.internet.com/Convertor1/

I am seeing this with the latest flash plugin as tested on:
http://www.macromedia.com/ (search field)
http://www.flash-db.com/SendMail/ (login field)

And with Shockwave on:
http://www.isketch.net/isketch.shtml
http://www.variableaspect.com/experiments/dircchat/

Couldn't dig up any interactive quicktime or other plugin based media to test
with... but happy to test if anyone wants to link up some samples.

Comment 11

[Peter Lubczynski]

Attachment: possible patch (v.1)

ok, talking with bryner, the problem appears that we are sending two keyDown
events now. The first keyDown comes from the KeyDown event (recently fixed) and
the second keyDown comes from the KeyPress event. The KeyPress event is
synthesized from the keyDown event.

This patch attempts to fix the problem by filtering out keyDown events on these
synthesized KeyPress events.

Comment 12

[Samir Gehani]

adt: nsbeta1+/adt1

Please land this on the 1.4 branch once you have reviews and drivers' approval.

Comment 13

[Simon Fraser [no longer active]]

I think brade made that change.

In the patch, don't assuem that uiEvent->nativeMsg is non-null. In Camino, we
synthesise some events, and don't always fill in the nativeMsg field.

Comment 14

[Peter Lubczynski]

Attachment: patch w/ null check (v.2)

ah, here's the same patch checking for a null nativeMsg.

Comment 15

[Brian Ryner (not reading)]

Comment on attachment 125869 [details] [diff] [review]
patch w/ null check (v.2)

>+    if (guiEvent &&
>+        guiEvent->message == NS_KEY_PRESS &&
>+        ev &&
>+        ev->what == keyDown)
>+      return NS_ERROR_FAILURE;
>+  }

I'd return NS_OK instead of NS_ERROR_FAILURE.  We didn't "fail" to do anything,
there was simply nothing to do.  Plus, I'm worried about this causing an
exception if JS dispatched the event (though I guess that wouldn't have the
event->what == keyDown).

sr=bryner with that change.

Comment 16

[Kathleen :Brade]

Comment on attachment 125869 [details] [diff] [review]
patch w/ null check (v.2)

r=brade with changes from bryner; be sure to test key-repeat and make sure that
still works (it seems like it should)

Comment 17

[Peter Lubczynski]

changed return value to NS_OK and checked in patch on the trunk (in today's
builds), marking FIXED.

Comment 18

[Peter Lubczynski]

Comment on attachment 125869 [details] [diff] [review]
patch w/ null check (v.2)

requesting drivers approval: this is a bad recent regression (since 1.3) for
plugins on Mac causing double key strokes for Flash.

Comment 19

[Asa Dotzler [:asa]]

Comment on attachment 125869 [details] [diff] [review]
patch w/ null check (v.2)

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 20

[Chris Casciano]

mozilla 2003061903 trunk os x.... something is still a bit funny

i'm only getting one char with the search form on macromedia.com but
type-ahead-find is fighting for my keystroke so i can only ever type one letter
before focus is lost. same goes for the other flash example.

on istetch things work fine but that's just cause my keystrokes aren't found in
the page and therefore focus is never lost. I didn't see this problem in older
builds.

The java applet examples linked don't seem to suffer from this problem.

Also, i don't know if its related or not, but some of the linked plugins won't
won't load with my old profile with today's build, but do with a new profile.
The macromedia site gives me the non-flash version and the java applets i linked
earlier don't display. All other lined pages work fine and they all do with a
clean profile. 

Comment 21

[Chris Petersen]

Confirming the same problem described in Comment 20#. Tested with 2003-06-19-03
Macho trunk under 10.2.6.

Comment 22

[Chris Petersen]

Attachment: Testcase using flash object and separate anchor link

Comment 23

[Chris Petersen]

Using the test case provided, I cannot reproduce this problem with the
2003-06-19-08 1.4 (branch). Placing carat in a field and typing "This is a test"
will still keep focus in the field. So basically this is fixed on the branch.

However, this focus problem is still reproducing on the Macho trunk
(2003-06-19-08). Placing carat in a field and starting to type "This is a test"
will place focus out of the field and onto the linked text.

Both the trunk and branch share the same user profile which have the
accessibility.typeaheadfind.autostart set to true.

Comment 24

[Peter Lubczynski]

I know what's going on, here's the problem:

on the trunk, I checked in the patch with the change in comment #15 to return
NS_OK while timeless checked in the orriginal patch that returns
NS_ERROR_FAILURE into the 1.4 branch. Returning NS_ERROR_FAILURE is the correct
thing to do in this case because the plugin wants the event consumed so things
like typeaheadfind don't take over. When my tree finishes updating and the
current bustage clears, I'll make sure the code is the same on the trunk as the
branch.

Comment 25

[Chris Casciano]

well... i can't verify the testcase... the linked swf is on an internal server
so its not much use to me.

I can, however verify that 2003061908 (1.4 branch) isn't suffering from the same
type ahead find conflicts as i earlier reported for 19-03 trunk and just now
retested and see in 19-08 trunk.

Comment 26

[Brian Ryner (not reading)]

It seems like a cleaner way to do that would be to call aKeyEvent->PreventDefault().

Comment 27

[Peter Lubczynski]

good idea...but all the other event listeners are returning NS_ERROR_FAILURE for
Mac. I've opened bug 210025 about changing them to return NS_OK and call
aKeyEvent->PreventDefault().

Comment 28

[Chris Casciano]

v. earlier branch and 2003062103 trunk os x (no more type ahead find
collisions). thx for the quick work.

Comment 29

[R.K.Aa.]

*** Bug 210256 has been marked as a duplicate of this bug. ***

Comment 30

[Asa Dotzler [:asa]]

mozilla1.4 shipped. unsetting blocking1.4 request.