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

Status: VERIFIED FIXED

(Core :: XUL, enhancement)

Description

[Alice0775 White]

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

Comment 1

[Lewis Rosenthal]

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

Comment 3

[Wladimir Palant]

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.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

Attachment: Patch

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.

Comment 5

[Neil Deakin]

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.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away until 10/14)]

(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 7

[[e-mail]]

Comment on attachment 8421105 [details] [diff] [review]
Patch

Wouldn't it have been easier to move the preventDefault calls into the _move* functions?

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/10c6ee2c7524

Comment 9

[Teodora Vermesan (:TeoVermesan)]

Reproduced the issue on Nightly (2012-07-26), verified as fixed on latest Aurora 32.0a2 (buildID: 20140711004006) under Windows 7 64bit.