Closed Bug 1714177 Opened 1 year ago Closed 1 year ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
91 Branch
Tracking Status
firefox-esr78 --- wontfix
firefox89 --- wontfix
firefox90 --- wontfix
firefox91 --- fixed

People

(Reporter: spohl, Assigned: spohl)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

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?

Flags: needinfo?(masayuki)

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 + ..

Flags: needinfo?(masayuki)

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

Assignee: nobody → spohl.mozilla.bugs
Status: NEW → ASSIGNED
Pushed by spohl@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 91 Branch

Can we land a test for this?

Flags: needinfo?(spohl.mozilla.bugs)

(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.

Flags: needinfo?(spohl.mozilla.bugs)
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.