Status

defect
P3
normal
VERIFIED FIXED
9 years ago
3 years ago

People

(Reporter: clokep, Assigned: raymondlee)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Blocks: 597043
Priority: -- → P3
Posted patch v1 (obsolete) — Splinter Review
For such a simple fix, what test should we write?  Ian: do you think we need a test for this patch?
Assignee: nobody → raymond
Attachment #487526 - Flags: feedback?(ian)
Attachment #487526 - Attachment is patch: true
Attachment #487526 - Attachment mime type: application/octet-stream → text/plain
Status: NEW → ASSIGNED
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.
Attachment #487526 - Flags: feedback?(ian) → feedback-
Posted patch v1 (obsolete) — Splinter Review
(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.
Attachment #487526 - Attachment is obsolete: true
Attachment #487872 - Flags: feedback?(ian)
Blocks: 609409
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.
Attachment #487872 - Flags: feedback?(ian) → feedback+
Attachment #487872 - Flags: review?(dolske)
Attachment #487872 - Flags: review?(dolske) → review+
Attachment #487872 - Flags: approval2.0?
Does it need a try run?
Sent it to try 00e2acb70818, waiting for the result.
(In reply to comment #6)
> Sent it to try 00e2acb70818, waiting for the result.

Passed try
Duplicate of this bug: 611989
Could someone give approval2.0 to this please?
OS: Windows 7 → All
Comment on attachment 487872 [details] [diff] [review]
v1

a=beltzner
Attachment #487872 - Flags: approval2.0? → approval2.0+
Attachment #487872 - Attachment is obsolete: true
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/b27ab9bbc12e
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Sent to try and passed with intermittent orange
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/fa69cf56d15b
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Works great, thanks! Now just need to get bug 609409 fixed for this to be very useful.
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.