1714177 - Spelling error in TextInputHandler causes branch to never execute (unused code?)

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Stephen A Pohl [:spohl]]

There is a spelling error (s/cancelOperatiorn:/cancelOperation:/) that causes that branch to never execute. Based on the preceding comment, this branch only seems to be necessary when ChildView is implementing cancelOperation:, which it currently doesn't seem to do. Masayuki, is this a simple case of correcting the spelling? Has this never been an issue because ChildView wasn't implementing cancelOperation:? Or is there something else that we're missing?

Comment 1

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

Honestly, I don't remember about it.

According to the commit message:

Note that this patch has a hack for cancelOperation: command.
The command is typically fired for Escape key press. However,
it's also fired for Command + Period. Unfortunately, this
behavior is really odd if subclass of NSResponder implements
|void cancelOperation:(id)sender|. If it's implemented,
Cocoa doesn't call its |void keyDown:(NSEvent)theEvent|.
Instead, it calls only |void doCommandBySelector:(SEL)aSelector|
and |void cancelOperation:(id)sender| when Command + Period is
pressed. Therefore, we cannot dispatch keydown nor keypress
event for this key combination if we implement it. Therefore,
this patch doesn't implement the method but handle it in
doCommandBySelector even though the super class of ChildView
cannot handle the command with this path.

It's implemented for consistent behavior at pressing Cmd + ..

Comment 2

[Stephen A Pohl [:spohl]]

Then maybe the best path forward is to simply correct the spelling for now. I'm going to post a patch shortly. Thank you!

Comment 3

[Stephen A Pohl [:spohl]]

Attachment: Bug 1714177: Correct spelling error in TextInputHandler that could have prevented us from handling escape key or Command and Period key presses correctly if ChildView were to ever implement cancelOperation:. r=masayuki

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ec22351c9738
Correct spelling error in TextInputHandler that could have prevented us from handling escape key or Command and Period key presses correctly if ChildView were to ever implement cancelOperation:. r=masayuki

Comment 5

[Sandor Molnar]

https://hg.mozilla.org/mozilla-central/rev/ec22351c9738

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Can we land a test for this?

Comment 7

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

(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)

Can we land a test for this?

Once bug 1679109 is fixed, yes. I.e., for now, it's not possible.