Closed Bug 777832 Opened 7 years ago Closed 5 years ago

Editable Tree widgets should support Home/End key to jump caret, Home/End key doesn't work>Page Bookmarked>Edit Folder Name

Categories

(Core :: XUL, enhancement)

x86
Windows 7
enhancement
Not set

Tracking

()

VERIFIED FIXED
mozilla32

People

(Reporter: alice0775, Assigned: masayuki)

References

Details

Attachments

(1 file)

http://hg.mozilla.org/mozilla-central/rev/20db7c6d82cc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120726052803

See http://forums.mozillazine.org/viewtopic.php?f=9&t=2507135

Step To reproduce:
1. Ctrl + D
2. Page Bookmarked window show
3. Click on folder down arrow to expand.
4. Click on [New folder]
5. Edit folder name, pressing Home and End button

Actual Results:
 Nothing happens

Expected Results:
 Caret position should jump properly
I've just bumped into this under FireFTP, and Mime pointed me to Core (I should have looked to see that he was using the tree widget code). When renaming a file or directory, it is not possible to use home/end to navigate the edit box. Silly.

Cross ref: https://www.mozdev.org/bugs/show_bug.cgi?id=25168
Duplicate of this bug: 1007680
Repeating my analysis from bug 1007680:

The issue are the handlers for keypress/keydown event in tree.xml. The problem is that these handlers have "accel any" as their modifiers attribute meaning that they will apply even if no modifier keys are pressed. And they have preventdefault="true" meaning that they will unconditionally prevent the default action. Note that the functions called there like _moveToEdge() will actually check this._editingColumn and return early - but that doesn't matter, the event has been canceled already.

IMHO the best solution would be to call event.preventDefault() explicitly in these functions, only if they actually handled the event.
Blocks: abp
Attached patch PatchSplinter Review
ns*Editor handles keypress events. Although, they should handle keydown events for non-printable keys in the future, we need a lot of work for the change.

For now, all keydown handlers of <tree> should check if a column is in editing mode. In that case, <tree> shouldn't handle keydown events since actual focused element is the <tree>.inputField, i.e., nsPlaintextEditor.

I have no idea to test this bug. Testing shortcut key handling for editor is difficult because Linux and Mac use native key binding which depends on system settings especially the former case.
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Attachment #8421105 - Flags: review?(enndeakin)
Comment on attachment 8421105 [details] [diff] [review]
Patch

You could also remove the this._editingColumn checks from the top of each of the _moveByOffset, _moveByOffsetShift, ... functions as well, as they are now redundant.
Attachment #8421105 - Flags: review?(enndeakin) → review+
(In reply to Neil Deakin from comment #5)
> Comment on attachment 8421105 [details] [diff] [review]
> Patch
> 
> You could also remove the this._editingColumn checks from the top of each of
> the _moveByOffset, _moveByOffsetShift, ... functions as well, as they are
> now redundant.

Although, if some addons used the methods without _editingColumn check, it might cause buggy behavior. Perhaps, it must be rare because editable <tree> isn't so major.

https://hg.mozilla.org/integration/mozilla-inbound/rev/10c6ee2c7524
Comment on attachment 8421105 [details] [diff] [review]
Patch

Wouldn't it have been easier to move the preventDefault calls into the _move* functions?
https://hg.mozilla.org/mozilla-central/rev/10c6ee2c7524
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
QA Whiteboard: [good first verify]
Reproduced the issue on Nightly (2012-07-26), verified as fixed on latest Aurora 32.0a2 (buildID: 20140711004006) under Windows 7 64bit.
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify] → [good first verify] [testday-20140711]
Depends on: 1247476
No longer depends on: 1247476
You need to log in before you can comment on or make changes to this bug.