Apostrophe followed by a letter creates two apostrophes

RESOLVED FIXED in mozilla15

Status

()

Core
Widget: Cocoa
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: reuben, Assigned: masayuki)

Tracking

({regression})

Trunk
mozilla15
x86
Mac OS X
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
I'm not sure whether this is an Editor or Widget: Cocoa bug.

Steps to reproduce:
1) With a US International PC keyboard layout on OS X Lion, try to write |'s|

Actual results:
Two apostrophes are displayed |''| 

Expected results:
Apostrophe followed by consonant |'s|
(Reporter)

Updated

5 years ago
Severity: normal → critical
Keywords: regression, regressionwindow-wanted
(Reporter)

Comment 1

5 years ago
Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=762e95608da3&tochange=e794cef56df6

Suspicious changesets:
69fc5803d184	Masayuki Nakano — Bug 447757 part.1 Compute DOM keycode for non-printable key from table first r=karlt
29e512736f30	Masayuki Nakano — Bug 677252 part.1 Reimplement keycode computation in cocoa widget r=smaug+smichaud
Keywords: regressionwindow-wanted
(Reporter)

Updated

5 years ago
Component: Editor → Widget: Cocoa
QA Contact: editor → cocoa
tracking-firefox15: --- → +
Sure, regression of bug 677252.

NSEvent's |characters| for "s" key returns "'s". insertText is called twice one is for "'" and the other is for "s". Therefore, the old code sets "s" for charCode of the keypress event but the new code sets it "'" which is the first character of |characters|.

We can fix this bug easily, but I need to decide "how" to fix this bug in code level though.
Assignee: nobody → masayuki
Severity: critical → major
Status: NEW → ASSIGNED
tracking-firefox15: + → ---
Created attachment 627159 [details] [diff] [review]
Patch

confirming on tryserver.
Comment on attachment 627159 [details] [diff] [review]
Patch

I think, basically, it's wrong to use first character of characters of NSEvent for charCode of our DOM keypress event. But it's not matter for now since there is no other actual problem.

At handling insertText, we should honor the inserting text rather than current key event's characters. This patch makes that the caller (InsertText() in this case) can specify charCode before calling InitKeyEvent(). It's a little bit hacky, but I think that it's okay for now.

I think that we will need to change the key event handlers when we fix the relation between composition events and key events (current behavior doesn't base on D3E definition). Then, we can make them better.
Attachment #627159 - Flags: review?(smichaud)
Comment on attachment 627159 [details] [diff] [review]
Patch

This looks fine to me as an interim patch.  But here are some changes to your comments that (I think) make them easier to understand:

+   *                              NOTE: When aKeyEvent is a keypress event and
+   *                                    caller expects the event causes to input
+   *                                    a character, caller should set charCode
+   *                                    before calling this.  Then, the charCode
+   *                                    value isn't modified.

"When aKeyEvent is a keypress event and the caller expects that the event will cause a character to be input (say in an editor), the caller should set aKeyEvent.charCode before calling this.  Then charCode won't be modified."

+   *                              NOTE: If caller expects this event causes
+   *                                    to input a character, caller should
+   *                                    set the character before calling this.
+   *                                    Then, the charCode is never modified.

"If the caller expects this event to cause character input (say in an editor), the caller should set aKeyEvent.charCode before calling this.  Then charCode won't be modified."

+  // If the inputting character is printable character, we're expecting that
+  // the keypress event causes to input it on editor.  For ensuring it, set
+  // charCode before calling InitKeyEvent().

"If the text to be inserted is a single printable character, we expect that the keypress event will cause it to be input in an editor.  To ensure this happens, set charCode before calling InitKeyEvent()."

+  PRUnichar inputtingChar = str.CharAt(0);

I think it makes sense to change "inputtingChar" to "insertedChar".
Attachment #627159 - Flags: review?(smichaud) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/85e529ae09f9
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/85e529ae09f9
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Depends on: 759524
Depends on: 775414
You need to log in before you can comment on or make changes to this bug.