Open
Bug 97434
Opened 23 years ago
Updated 2 years ago
Some Windows keyboard behaviours missing from trees
Categories
(Core :: XUL, enhancement)
Tracking
()
ASSIGNED
mozilla1.0.1
People
(Reporter: neil, Assigned: neil)
References
(Depends on 1 open bug)
Details
Attachments
(5 files, 3 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
janv
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
janv
:
review+
alecf
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
mconnor
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•23 years ago
|
||
Comment 2•23 years ago
|
||
In some cases we're deliberately deviating from the platform.
Can I see a patch (a diff)?
Assignee | ||
Comment 3•23 years ago
|
||
Assignee | ||
Comment 4•23 years ago
|
||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.0
Updated•23 years ago
|
Target Milestone: mozilla1.0 → mozilla1.0.1
Comment 5•23 years ago
|
||
Neil: Does this patch still apply? Have you asked for review/super-review yet?
Assignee | ||
Comment 6•23 years ago
|
||
It's bitrotted rather badly because outliners are now trees :-) Resummarizing.
Summary: Outliner key bindings don't match Windows platform behaviour. → Some Windows keyboard behaviours missing from trees
Comment 7•22 years ago
|
||
*** Bug 146620 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 8•22 years ago
|
||
Attachment #47489 -
Attachment is obsolete: true
Attachment #47652 -
Attachment is obsolete: true
Attachment #48291 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #114026 -
Flags: review?(varga)
Assignee | ||
Comment 9•22 years ago
|
||
Comment on attachment 114026 [details] [diff] [review]
New patch
I've just noticed that treeSelection handles single selection too, sigh...
Attachment #114026 -
Attachment is obsolete: true
Attachment #114026 -
Flags: review?(varga)
Assignee | ||
Comment 10•22 years ago
|
||
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).
Attachment #114026 -
Attachment is obsolete: false
Attachment #114026 -
Flags: review?(varga)
Comment 11•22 years ago
|
||
*** Bug 193915 has been marked as a duplicate of this bug. ***
Comment 12•22 years ago
|
||
See also bug 135749.
Assignee | ||
Comment 13•21 years ago
|
||
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.
Assignee: hyatt → neil.parkwaycc.co.uk
Assignee | ||
Updated•21 years ago
|
Attachment #147299 -
Flags: review?(varga)
Comment 14•21 years ago
|
||
Comment on attachment 147299 [details] [diff] [review]
Kill selectionHead/Tail portion of patch
Awesome!
r=varga
Attachment #147299 -
Flags: review?(varga) → review+
Assignee | ||
Comment 15•21 years ago
|
||
Comment on attachment 147299 [details] [diff] [review]
Kill selectionHead/Tail portion of patch
I think alecf likes these cruft deletion patches :-)
Attachment #147299 -
Flags: superreview?(alecf)
Comment 16•21 years ago
|
||
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
Attachment #147299 -
Flags: superreview?(alecf) → superreview+
Assignee | ||
Comment 17•21 years ago
|
||
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 :-[
Assignee | ||
Comment 18•21 years ago
|
||
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.
Assignee | ||
Updated•21 years ago
|
Attachment #149201 -
Flags: review?(varga)
Comment 19•21 years ago
|
||
Comment on attachment 149201 [details] [diff] [review]
Handles most of the keys
looks good
r=varga
Attachment #149201 -
Flags: review?(varga) → review+
Assignee | ||
Updated•21 years ago
|
Attachment #149201 -
Flags: superreview?(alecf)
Comment 20•21 years ago
|
||
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...
Attachment #149201 -
Flags: superreview?(alecf) → superreview+
Assignee | ||
Comment 21•21 years ago
|
||
(In reply to comment #20)
>you actually even REMOVED a comment from vk_home..
Doh :-[
Comment 22•21 years ago
|
||
sorry, I'm the one responsible for those variables in the original code :(
Assignee | ||
Comment 23•21 years ago
|
||
Assignee | ||
Comment 24•21 years ago
|
||
Comment on attachment 149207 [details] [diff] [review]
With comments
Checked in.
Comment 25•20 years ago
|
||
(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.
Assignee | ||
Comment 26•20 years ago
|
||
(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...
Updated•20 years ago
|
Attachment #114026 -
Flags: review?(varga)
Comment 27•20 years ago
|
||
ports attachment 149207 [details] [diff] [review] to toolkit/
Updated•20 years ago
|
Attachment #160825 -
Flags: review?(mconnor)
Comment 28•20 years ago
|
||
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.
Attachment #160825 -
Flags: review?(mconnor) → review+
Updated•20 years ago
|
Attachment #160825 -
Attachment description: toolkit port → toolkit port (checked in)
Comment 29•20 years ago
|
||
I suspect the toolkit port needs relanding, although I haven't verified
Keywords: aviary-landing
Comment 30•20 years ago
|
||
Ben backed out his checkin. rev1.14 is the same as rev1.12 (from this bug).
Keywords: aviary-landing
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•