Closed Bug 955326 Opened 10 years ago Closed 10 years ago

Tab completion does not always update the character counter

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

References

Details

(Whiteboard: [1.4-wanted])

Attachments

(1 file, 2 obsolete files)

*** Original post on bio 1891 at 2013-03-09 18:14:00 UTC ***

Due to early returns from the keypress handler.
*** Original post on bio 1891 at 2013-04-09 15:06:58 UTC ***

NOT fixed by bug 953705 (bio 260). In fact it's now worse. Needs investigating.
Assignee: nobody → aleth
Whiteboard: [1.4-wanted]
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1891 as attmnt 2337 at 2013-04-10 12:09:00 UTC ***

Adds missing inputInputHandler calls where editor.value is modified directly.
Attachment #8354104 - Flags: review?(benediktp)
Blocks: 953705
*** Original post on bio 1891 at 2013-04-10 12:17:56 UTC ***

Comment on attachment 8354104 [details] [diff] [review] (bio-attmnt 2337)
Patch

>diff --git a/instantbird/content/conversation.xml b/instantbird/content/conversation.xml
>       <method name="addPrompt">
>         <parameter name="aPrompt"/>
>         <body>
>         <![CDATA[
>           let editor = this.editor;
>           let currentEditorValue = editor.value;
>           if (currentEditorValue.indexOf(aPrompt) != 0)
>             editor.value = aPrompt + currentEditorValue;
>           editor.focus();
>+          this.inputInputHandler();          
>         ]]>
>         </body>
>       </method>

Drive by nit: trailing white space alert!
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1891 as attmnt 2338 at 2013-04-10 12:23:00 UTC ***

Nit fix
Attachment #8354105 - Flags: review?(benediktp)
Comment on attachment 8354104 [details] [diff] [review]
Patch

*** Original change on bio 1891 attmnt 2337 at 2013-04-10 12:23:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354104 - Attachment is obsolete: true
Attachment #8354104 - Flags: review?(benediktp)
Comment on attachment 8354105 [details] [diff] [review]
Patch

*** Original change on bio 1891 attmnt 2338 at 2013-04-10 20:24:47 UTC ***

With your patch applied it is working well now :)

Only thing: please rename "inputInputHandler" (and "delayedInputHandler") to something more reasonable. It was a bad choice of name before already but now that it is called directly from other places, it makes even less sense:

>           this.editor.value = aConversation.editor.value;
>           this.browser.browserResize();
>           this.updateTyping();
>+          this.inputInputHandler();
>           this.loaded = true;
>           this.observe(this.browser, "conversation-loaded", null);

I can somewhat imagine what the other calls are doing here but "inputInputHandler()" is confusing :(

I'm uncertain what a better name would be. Something with "handle"/"update"/"input"/"change", maybe?

I'll r+ this and you can carry it forward when attaching a patch with a new name.
Attachment #8354105 - Flags: review?(benediktp) → review+
*** Original post on bio 1891 at 2013-04-10 20:31:49 UTC ***

I forgot: inputInputHandler has a parameter "event" which is never used. Would you remove it while you're at it?
Attached patch PatchSplinter Review
*** Original post on bio 1891 as attmnt 2345 at 2013-04-11 11:33:00 UTC ***

How about this?
Attachment #8354112 - Flags: review?(benediktp)
Comment on attachment 8354105 [details] [diff] [review]
Patch

*** Original change on bio 1891 attmnt 2338 at 2013-04-11 11:33:42 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354105 - Attachment is obsolete: true
Comment on attachment 8354112 [details] [diff] [review]
Patch

*** Original change on bio 1891 attmnt 2345 at 2013-04-13 21:19:02 UTC ***

Works fine with replies on Twitter, completion and when dragging a tab into a new window. Thanks for fixing this!
Attachment #8354112 - Flags: review?(benediktp) → review+
Whiteboard: [1.4-wanted] → [1.4-wanted][checkin-needed]
*** Original post on bio 1891 at 2013-04-13 21:44:46 UTC ***

http://hg.instantbird.org/instantbird/rev/9368825cc2b4

thanks!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [1.4-wanted][checkin-needed] → [1.4-wanted]
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.