Closed Bug 1131026 Opened 5 years ago Closed 5 years ago

nsITextInputProcessor.init(ForTests) should be renamed to nsITextInputProcessor.beginInputTransaction(ForInit)

Categories

(Core :: User events and focus handling, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: masayuki, Assigned: masayuki)

References

Details

(Keywords: dev-doc-complete, inputmethod)

Attachments

(1 file)

Attached patch PatchSplinter Review
I agree with your idea. The names are a little bit long, but it's not so problem.

Similarly, "notify-detached" should be renamed to "notify-end-input-transaction".
Attachment #8561377 - Flags: superreview?(bugs)
Attachment #8561377 - Flags: review?(bugs)
Comment on attachment 8561377 [details] [diff] [review]
Patch

>+   * initForTest(), first.  See beginInputTransaction() for more detail of this.
s/initForTest/beginInputTransactionForTests/


So it is not clear to me whether we need this, if init() just worked as expected.
But either way.
Attachment #8561377 - Flags: superreview?(bugs)
Attachment #8561377 - Flags: superreview+
Attachment #8561377 - Flags: review?(bugs)
Attachment #8561377 - Flags: review+
(In reply to Olli Pettay [:smaug] from comment #2)
> Comment on attachment 8561377 [details] [diff] [review]
> Patch
> 
> >+   * initForTest(), first.  See beginInputTransaction() for more detail of this.
> s/initForTest/beginInputTransactionForTests/

Oops, thanks! (I used replace of my editor but the comment misspelled...)

> So it is not clear to me whether we need this, if init() just worked as
> expected.
> But either way.

The method name, begin input transaction, sounds better to me because:

* the name is clearer what they do.
* the notification name is also clearer, especially the relation between the notification and these methods.
* it's better name to grep the method ;-) (init() is defined by a lot of classes!)

I'll land it, but if you have additional objections, I'll back it out.
https://hg.mozilla.org/mozilla-central/rev/795e5c73a3a6
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.