Closed Bug 954844 Opened 10 years ago Closed 10 years ago

[Tab Complete] Pressing tab while cursor is in a nickname produces garbage

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benediktp, Assigned: aleth)

Details

Attachments

(2 files, 2 obsolete files)

*** Original post on bio 1410 at 2012-04-28 19:05:00 UTC ***

STR:
1. Complete a nickname.
2. Move cursor into the name.
3. Press tab again.

Example result:
al|eth: -> aleth: eth:

I think in this case we should just ignore that the tab key was pressed or, if we choose to complete, replace the old nickname completely instead.
I've no opinion on the case where the cursor is in a word that is not a nickname.
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1410 as attmnt 1412 at 2012-04-28 21:56:00 UTC ***

Thanks for spotting this edge case. 

Really I suspect the user in this case would 'like' to continue cycling, but of course it's impossible to know from which letter onwards that particular nick was completed in the past (if at all).

I considered not moving the cursor, but that might just lead to the user pressing TAB a couple times, wondering if it's broken.

Review question: Is toString() the recommended function to use here?
Attachment #8353164 - Flags: review?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment on attachment 8353164 [details] [diff] [review]
Patch

*** Original change on bio 1410 attmnt 1412 at 2012-04-28 23:22:20 UTC ***

>diff --git a/instantbird/content/conversation.xml b/instantbird/content/conversation.xml

>+          let wholeWord =
>+            text.substring(inputBox.selectionStart - word.length).match(/^[\w-]+/).toString();

I think you want to use [0] instead of .toString(), as match has no reason to return null, given that we return early if word is empty.

>+          if (matchingCompletions.indexOf(wholeWord) != -1) {
>+            inputBox.selectionStart += wholeWord.length - word.length;

You may want to move the cursor to the end of the completion rather than the end of the completed nick, so when the nick is a the beginning of a line, that would make the cursor move after the ", " or ": ".
Attachment #8353164 - Flags: review?(florian) → review-
Attached patch PatchSplinter Review
*** Original post on bio 1410 as attmnt 1413 at 2012-04-28 23:29:00 UTC ***

Also jumps past subsequent , and : to mimic completion behaviour.
Attachment #8353165 - Flags: review?(florian)
Comment on attachment 8353164 [details] [diff] [review]
Patch

*** Original change on bio 1410 attmnt 1412 at 2012-04-28 23:29:25 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353164 - Attachment is obsolete: true
Comment on attachment 8353165 [details] [diff] [review]
Patch

*** Original change on bio 1410 attmnt 1413 at 2012-04-28 23:33:14 UTC ***

Looks good, thanks! (I haven't tried it though.)
Attachment #8353165 - Flags: review?(florian) → review+
*** Original post on bio 1410 at 2012-04-29 23:08:09 UTC ***

Committed as http://hg.instantbird.org/instantbird/rev/03b51dc961be
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
Attached patch Patch to fix bustage (obsolete) — Splinter Review
*** Original post on bio 1410 as attmnt 1426 at 2012-05-02 12:10:00 UTC ***

Fix broken command completion (where the word contains the slash character). We don't need to worry about / only appearing at the beginning as this isn't the code to check whether we are completing something valid.
Attachment #8353178 - Flags: review?(florian)
*** Original post on bio 1410 as attmnt 1427 at 2012-05-02 12:57:00 UTC ***

Also fixes the fact that this shouldn't abort the completion if we are at the end of the word even if that word is a valid nick ("/j<tab>" should show two possible completions in IRC, "Even<tab>" should notice that Even1 exists, etc).
Attachment #8353179 - Flags: review?(florian)
Comment on attachment 8353178 [details] [diff] [review]
Patch to fix bustage

*** Original change on bio 1410 attmnt 1426 at 2012-05-02 12:57:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353178 - Attachment is obsolete: true
Attachment #8353178 - Flags: review?(florian)
Comment on attachment 8353179 [details] [diff] [review]
Patch to fix bustage

*** Original change on bio 1410 attmnt 1427 at 2012-05-02 13:38:18 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353179 - Flags: review?(florian) → review+
*** Original post on bio 1410 at 2012-05-03 17:37:41 UTC ***

http://hg.instantbird.org/instantbird/rev/8686ea6557e5
You need to log in before you can comment on or make changes to this bug.