Closed Bug 467593 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: DOM: Events, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: fixed1.9.1, late-l10n)

Attachments

(1 file, 3 obsolete files)

Attached patch Patch v1.0 (obsolete) — Splinter Review
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.
Attachment #350998 - Flags: superreview?(roc)
Attachment #350998 - Flags: review?(roc)
Should we report an error to the error console instead?
(In reply to comment #1)
> Should we report an error to the error console instead?

always? or only chrome code?
Always, so that Web authors can see it.
Attached patch Patch v2.0 (obsolete) — Splinter Review
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?
Attachment #350998 - Attachment is obsolete: true
Attachment #351127 - Flags: superreview?(roc)
Attachment #351127 - Flags: review?(roc)
Attachment #350998 - Flags: superreview?(roc)
Attachment #350998 - Flags: review?(roc)
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.
Keywords: late-l10n
(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.
Attached patch Patch v2.1 (obsolete) — Splinter Review
Attachment #351127 - Attachment is obsolete: true
Attachment #351183 - Flags: superreview?(roc)
Attachment #351183 - Flags: review?(roc)
Attachment #351127 - Flags: superreview?(roc)
Attachment #351127 - Flags: review?(roc)
Attached patch Patch v2.1Splinter Review
sorry for the spam.
Attachment #351183 - Attachment is obsolete: true
Attachment #351184 - Flags: superreview?(roc)
Attachment #351184 - Flags: review?(roc)
Attachment #351183 - Flags: superreview?(roc)
Attachment #351183 - Flags: review?(roc)
Attachment #351184 - Flags: superreview?(roc)
Attachment #351184 - Flags: superreview+
Attachment #351184 - Flags: review?(roc)
Attachment #351184 - Flags: review+
Attachment #351184 - Flags: approval1.9.1?
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 on attachment 351184 [details] [diff] [review]
Patch v2.1

Note, this has l10n change
landed on trunk.
http://hg.mozilla.org/mozilla-central/rev/c8ca4434292a
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Whiteboard: [c-n: baking for 1.9.1]
Comment on attachment 351184 [details] [diff] [review]
Patch v2.1

a191=beltzner
Attachment #351184 - Flags: approval1.9.1? → approval1.9.1+
Duplicate of this bug: 516615
Blocks: 581850
You need to log in before you can comment on or make changes to this bug.