618816 - Character by character selection using the mouse in the tab group name field.

Status: VERIFIED FIXED

Description

[Andreea Pod]

User-Agent:       Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre
Build Identifier: Mozilla/5.0 (X11; Linux i686; rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre

After setting the name of a group you are not able to select characters or to change the cursor's position using the mouse in the field. Since that is a text box the mouse should work as it normally does.

Reproducible: Always

Steps to Reproduce:
1.Go to group view.
2.Name one of the groups that you have and hit enter.
3.Click again in the text field and try to select with the mouse one or few characters.
Actual Results:  
The text can't be selected.

Expected Results:  
You should be able to select or move the cursor using the mouse.

Comment 1

[aravindm]

Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101213 Firefox/4.0b8pre

Able to reproduce. Unable to select few characters with the mouse when highlighting on the name field in panorama view. After clicking on the group name field in panorama view, entire text is selected.

Comment 2

[juan becerra [:juanb]]

This forces you to use the arrow keys if you want to position the cursor within the group name to edit it. It also happens on Mac. Nominating because it's very noticeable and it might be a good polish fix to have in the odd chance it is easy to fix.

Comment 3

[Shawn Wilsher :sdwilsh]

We wouldn't hold the release for this, but we'd take a patch with unit tests.  You can still edit the name by either deleting characters or using the keyboard arrows.  You can also click into a specific spot where you want to edit.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Created attachment 508359 [details] [diff] [review]
patch v1

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Passed try.

Comment 6

[Ian Gilman [:iangilman]]

Comment on attachment 508359 [details] [diff] [review]
patch v1

>   this.$title
>     .blur(function() {
>+      delete this.focused;
>       self.$titleShield.show();
>     })
>     .focus(function() {
>-      (self.$title)[0].select();
>+      if (!this.focused) {
>+        (self.$title)[0].select();
>+        this.focused = true;
>+      }
>+    })

I feel a little funny about just adding a property named something as common as "focused" to a DOM element... I suppose it'll be fine now, but you never know what it might collide with in the future. I'd rather go with a property of the GroupItem itself, like so: 

>   this._titleFocused = false;
>   this.$title
>     .blur(function() {
>+      self._titleFocused = false;
>       self.$titleShield.show();
>     })
>     .focus(function() {
>-      (self.$title)[0].select();
>+      if (!self._titleFocused) {
>+        (self.$title)[0].select();
>+        self._titleFocused = true;
>+      }
>+    })

Meanwhile, in the test:

>+  let createGroupItem = function () {
>+    let bounds = new cw.Rect(20, 20, 400, 200);
>+    let groupItem = new cw.GroupItem([], {bounds: bounds, immediately: true});
>+
>+    let groupItemId = groupItem.id;
>+    registerCleanupFunction(function() {
>+      let groupItem = cw.GroupItems.groupItem(groupItemId);
>+      if (groupItem)
>+        groupItem.close();
>+    });
>+
>+    return groupItem;
>+  }

Is it time to put some version of this in head.js? Doesn't need to happen with this patch, I suppose, but soon?

R+ with the variable change.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

Created attachment 508949 [details] [diff] [review]
patch v2

(In reply to comment #6)
> I feel a little funny about just adding a property named something as common as
> "focused" to a DOM element...

Fixed. Weird that such a property doesn't exist yet :)

> >+  let createGroupItem = function () {
> >+  }
> 
> Is it time to put some version of this in head.js? Doesn't need to happen with
> this patch, I suppose, but soon?

I thought about this lately, too. Let's do that in the next patch.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Created attachment 509311 [details] [diff] [review]
patch for checkin

Comment 9

[:Ehsan Akhgari]

http://hg.mozilla.org/mozilla-central/rev/be4f8b47377e

Comment 10

[George Carstoiu]

Mozilla/5.0 (Windows NT 6.1; rv:2.0b12pre) Gecko/20110215 Firefox/4.0b12pre

Issue verified.