I went through all the outliner key bindings and compared their effect to Windows. The Windows behaviour, where it differs, is as follows: Only empty trees are allowed to have a current index of -1. Left selects the parent of a leaf or collapsed node. Right selects the first child of an expanded parent.Shift+Direction selects from the last anchor or else the current item. Shift+Up/Down has no effect at the beginning/end, but Shift+Home/End/Page Up/Down always selects the current item. Home/End always ensures that the end item is visible, even if already current. Ctrl+Space toggles the selection state of the current item. For some reason Windows permits this on single-selection trees which otherwise ignore modifiers on events.

In some cases we're deliberately deviating from the platform. Can I see a patch (a diff)?

Neil: Does this patch still apply? Have you asked for review/super-review yet?

It's bitrotted rather badly because outliners are now trees :-) Resummarizing.

*** Bug 146620 has been marked as a duplicate of this bug. ***

Comment on attachment 114026 [details] [diff] [review] New patch I've just noticed that treeSelection handles single selection too, sigh...

Comment on attachment 114026 [details] [diff] [review] New patch Changed my mind; treeSelection's single selection code isn't useful enough (maybe it would be useful as an attribute).

*** Bug 193915 has been marked as a duplicate of this bug. ***

See also bug 135749.

There are a few tweaks to make the trees behave more along Windows lines e.g. if you press Home, Shift+End, End then only the last item will be selected.

Comment on attachment 147299 [details] [diff] [review] Kill selectionHead/Tail portion of patch Awesome! r=varga

Comment on attachment 147299 [details] [diff] [review] Kill selectionHead/Tail portion of patch I think alecf likes these cruft deletion patches :-)

Comment on attachment 147299 [details] [diff] [review] Kill selectionHead/Tail portion of patch I realize you're removing code, but it sure would be nice to get some comments on the single-line replacements.. sr=alecf

Comment on attachment 147299 [details] [diff] [review] Kill selectionHead/Tail portion of patch Checked in but also killing an extra assignment to selectionHead/Tail that I omitted here :-[

This still doesn't quite handle PgUp/Dn quite how I like them :-/ In particular I think [Ctrl+]Shift+PgUp should extend/select to the first visible row, unless that is already the current row, in which case it should scroll up a page first.

Comment on attachment 149201 [details] [diff] [review] Handles most of the keys looks good r=varga

Comment on attachment 149201 [details] [diff] [review] Handles most of the keys how about some comments here? these are especially hard to read with variables named "l" and so forth. I mean, you tell me if this is easy to understand: + if (l < 0) + return; + if (!event.ctrlKey) + this.view.selection.timedSelect(l, this._selectDelay); + else if (!this.view.selection.single) + this.currentIndex = l; this.treeBoxObject.ensureRowIsVisible(l); you actually even REMOVED a comment from vk_home.. sr=alecf with some comments...

(In reply to comment #20) >you actually even REMOVED a comment from vk_home.. Doh :-[

sorry, I'm the one responsible for those variables in the original code :(

Comment on attachment 149207 [details] [diff] [review] With comments Checked in.

(In reply to neil@parkwaycc.co.uk, comment #24) > (From update of attachment 149207 [details] [diff] [review]) > Checked in. Is there any reason why this bug is still open? Prog.

(In reply to comment #25) >Is there any reason why this bug is still open? Because some of the keys still don't work the Windows way...

ports attachment 149207 [details] [diff] [review] to toolkit/

Comment on attachment 160825 [details] [diff] [review] toolkit port (checked in) >+ <handler event="keypress" keycode="vk_end" modifiers="control any"> > <![CDATA[ > var l = this.view.rowCount - 1; >- this.view.selection.timedSelect(l, this._selectDelay); >+ if (l < 0) >+ return; >+ // Normal behaviour is to select the last row >+ if (!event.ctrlKey) >+ this.view.selection.timedSelect(l, this._selectDelay); >+ // In a multiselect tree Ctrl+End moves the anchor >+ else if (!this.view.selection.single) >+ this.currentIndex = l; > this.treeBoxObject.ensureRowIsVisible(l); > ]]> fix the whitespace here, please, otherwise, looks good to me.

I suspect the toolkit port needs relanding, although I haven't verified

Ben backed out his checkin. rev1.14 is the same as rev1.12 (from this bug).