467593 - "WARNING: GetCharCode used for wrong key event; should use onkeypress." should not be displayed by Web pages

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v1.0

spun off from bug 465461 comment 11.

In gmail text field of mail composing, every key typing display following warning:

> WARNING: GetCharCode used for wrong key event; should use onkeypress.: file
> /Users/josh/src/mozilla/ff_192_debug/content/events/src/nsDOMKeyboardEvent.cpp,
> line 116

nsDOMKeyboardEvent::GetCharCode outputs this warning when the event is not keypress. It's good for finding our bugs. But it should not be displayed by Web apps.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should we report an error to the error console instead?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to comment #1)
> Should we report an error to the error console instead?

always? or only chrome code?

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Always, so that Web authors can see it.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.0

ok. But I have a worry. Doesn't this regress the performance on Gmail (and also other web apps which have same error)?

And should I add NS_WARNING or something to nsDOMEvent::ReportWrongPropertyAccessWarning for our bugs which will be displayed on the terminal of debug build?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm not worried about performance if this only fires once per keystroke.

+  nsAutoString propertyName, type;
+  GetType(type);
+  propertyName.AssignWithConversion(aPropertyName)

You could just use AssignASCII.

+WrongEventPropertyAccessWarning=%S property of %S event should not be used. The value is always worthless value.

How about

%S property of %S event should not be used. The value is always worthless value.

The '%S' property of a %S event should not be used. The value is meaningless.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #4)
> And should I add NS_WARNING or something to
> nsDOMEvent::ReportWrongPropertyAccessWarning for our bugs which will be
> displayed on the terminal of debug build?

I don't think so.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.1

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Patch v2.1

sorry for the spam.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 351184 [details] [diff] [review]
Patch v2.1

just outputs warnings to error console when web apps access to charCode/keyCode/which properties which are not return meaningful value. This change helps web app developers.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 351184 [details] [diff] [review]
Patch v2.1

Note, this has l10n change

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

landed on trunk.
http://hg.mozilla.org/mozilla-central/rev/c8ca4434292a

Comment 12

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 351184 [details] [diff] [review]
Patch v2.1

a191=beltzner

Comment 13

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/1dd04166321c