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

VERIFIED FIXED in Firefox 4.0b12

Status

Firefox Graveyard
Panorama
P4
enhancement
VERIFIED FIXED
8 years ago
2 years ago

People

(Reporter: Andreea Pod, Assigned: ttaubert)

Tracking

Trunk
Firefox 4.0b12
Bug Flags:
in-testsuite +

Details

(Whiteboard: [4b8])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 1

8 years ago
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.
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.
blocking2.0: --- → ?
Whiteboard: [4b8]
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.
blocking2.0: ? → -
Component: General → TabCandy
QA Contact: general → tabcandy
Blocks: 627096
Priority: -- → P4
(Assignee)

Updated

7 years ago
Assignee: nobody → tim.taubert
OS: Linux → All
Hardware: x86 → All
Version: unspecified → Trunk
(Assignee)

Comment 4

7 years ago
Created attachment 508359 [details] [diff] [review]
patch v1
Attachment #508359 - Flags: review?(ian)
(Assignee)

Updated

7 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 5

7 years ago
Passed try.
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.
Attachment #508359 - Flags: review?(ian) → review+
(Assignee)

Updated

7 years ago
Attachment #508359 - Flags: approval2.0?
(Assignee)

Updated

7 years ago
Attachment #508359 - Flags: approval2.0?
(Assignee)

Comment 7

7 years ago
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.
Attachment #508359 - Attachment is obsolete: true
Attachment #508949 - Flags: approval2.0?

Updated

7 years ago
Attachment #508949 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 8

7 years ago
Created attachment 509311 [details] [diff] [review]
patch for checkin
Attachment #508949 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Keywords: checkin-needed

Comment 9

7 years ago
http://hg.mozilla.org/mozilla-central/rev/be4f8b47377e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b12

Comment 10

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

Issue verified.
Status: RESOLVED → VERIFIED
(Assignee)

Updated

7 years ago
Duplicate of this bug: 609409
Duplicate of this bug: 587250
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.