608714 - Unable to edit tab group names

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P3)

Description

[Patrick Cloke [:clokep]]

After naming a tab group in Panorama, I'm unable to edit a group name. When clicking anywhere in the name it will highlight the entire name and I must delete it and retype it, even to just fix a simple typo.

Further notes: when typing a group name, I'm unable to move the cursor from the end (arrow keys do not work and clicking with a mouse "sets" the title), to fix any typos I have to delete all the text back to the typo and retype.

Comment 1

[Raymond Lee [:raymondlee]]

Attachment: v1

For such a simple fix, what test should we write?  Ian: do you think we need a test for this patch?

Comment 2

[Ian Gilman [:iangilman]]

Comment on attachment 487526 [details] [diff] [review]
v1

This fix is good for the arrow keys, but it doesn't help with clicking. Currently, the first time you click the name (to start editing it), it selects the whole thing; this is good. However, once you're editing it and it's all selected like that, if you click a second time it in fact pops you out of editing mode rather than allowing you to put the insertion point where you clicked. I think it should instead put the insertion point there or even allow you to drag a selection (assuming you're already editing the text).

As for a test, I suppose we could skip it. I imagine if we did write a test it would be a bunch of simulated clicks and key strokes in the title area of the group, which seems doable.

Comment 3

[Raymond Lee [:raymondlee]]

Attachment: v1

(In reply to comment #2)
> Comment on attachment 487526 [details] [diff] [review]
> v1
> 
> This fix is good for the arrow keys, but it doesn't help with clicking.
> Currently, the first time you click the name (to start editing it), it selects
> the whole thing; this is good. However, once you're editing it and it's all
> selected like that, if you click a second time it in fact pops you out of
> editing mode rather than allowing you to put the insertion point where you
> clicked. I think it should instead put the insertion point there or even allow
> you to drag a selection (assuming you're already editing the text).
> 

If you click the area second time, it would remain in the edit mode now.  I was trying to make it allow you to put the insertion point where you clicked, however, it would require to re-write the whole title bar.  At the moment, if you click on that area to enter into the edit mode, you actually click on the title-shield div element, not the input element, and  then it manually call focus() on the input element.  I would suggest to file another bug to fix the remaining things in the future since there are many higher priority bugs to do.

Comment 4

[Ian Gilman [:iangilman]]

Comment on attachment 487872 [details] [diff] [review]
v1

Looks good.

(In reply to comment #3)
> If you click the area second time, it would remain in the edit mode now.  I was
> trying to make it allow you to put the insertion point where you clicked,
> however, it would require to re-write the whole title bar.  At the moment, if
> you click on that area to enter into the edit mode, you actually click on the
> title-shield div element, not the input element, and  then it manually call
> focus() on the input element.  I would suggest to file another bug to fix the
> remaining things in the future since there are many higher priority bugs to do.

Fair enough. Filed bug 609409.

Comment 5

[Ian Gilman [:iangilman]]

Does it need a try run?

Comment 6

[Raymond Lee [:raymondlee]]

Sent it to try 00e2acb70818, waiting for the result.

Comment 7

[Raymond Lee [:raymondlee]]

(In reply to comment #6)
> Sent it to try 00e2acb70818, waiting for the result.

Passed try

Comment 9

[Raymond Lee [:raymondlee]]

Could someone give approval2.0 to this please?

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 487872 [details] [diff] [review]
v1

a=beltzner

Comment 11

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 12

[Ian Gilman [:iangilman]]

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

Comment 13

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This bug was part of a mass backout to fix the permanent leak on OS X 64 that this push caused.

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

Comment 14

[Raymond Lee [:raymondlee]]

Sent to try and passed with intermittent orange

Comment 15

[(no longer active)]

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

Comment 16

[Patrick Cloke [:clokep]]

Works great, thanks! Now just need to get bug 609409 fixed for this to be very useful.