Add a mechanism to get the previous visible column in a tree and use that instead of the boxobject

RESOLVED FIXED in Firefox 67

Status

()

enhancement
P3
normal
RESOLVED FIXED
7 months ago
6 months ago

People

(Reporter: enndeakin, Assigned: enndeakin)

Tracking

unspecified
mozilla67
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments)

The tree is the only caller that uses the box order properties, used for column drag and drop (these properties return the elements in visible order rather than dom order). Instead, add a property to TreeColumns for this specific usecase.

Blocks: 1519948
No longer blocks: 1519957
Priority: -- → P3
Attachment #9036410 - Flags: review?(bzbarsky)
Comment on attachment 9036410 [details] [diff] [review]
Add a previousVisibleSibling property to TreeColumn instead of using box objects to support tree column drag and drop

>+  readonly attribute TreeColumn? previousVisibleSibling;

I'm not sure about the naming here, since it might not in fact be visible (e.g. could be width 0, no?).

>+++ b/layout/xul/tree/nsTreeColumns.cpp
>+already_AddRefed<nsTreeColumn> nsTreeColumn::GetPreviousVisibleSibling() {
>+        frame->GetContent() &&

No need; the only frame with a null GetContent() is the viewport.

>+      RefPtr<nsTreeColumn> column =
>+        mColumns->GetColumnFor(frame->GetContent()->AsElement());

So this walks elements backwards one by one and for each element walks the columns forward from the front, right?  Seems like it could have O(N^2) behavior in weird cases.  Do we need to worry about that?
Flags: needinfo?(enndeakin)
Comment on attachment 9036410 [details] [diff] [review]
Add a previousVisibleSibling property to TreeColumn instead of using box objects to support tree column drag and drop

Pending answers to comment 1...
Attachment #9036410 - Flags: review?(bzbarsky)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #1)

Comment on attachment 9036410 [details] [diff] [review]
Add a previousVisibleSibling property to TreeColumn instead of using box
objects to support tree column drag and drop

  • readonly attribute TreeColumn? previousVisibleSibling;

I'm not sure about the naming here, since it might not in fact be visible
(e.g. could be width 0, no?).

I will rename it previousSiblingInOrder.

  •  RefPtr<nsTreeColumn> column =
    
  •    mColumns->GetColumnFor(frame->GetContent()->AsElement());
    

So this walks elements backwards one by one and for each element walks the
columns forward from the front, right? Seems like it could have O(N^2)
behavior in weird cases. Do we need to worry about that?

I don't think this is an issue. Under any normal usage, it would iterate within GetColumnFor only twice (or once if there is no splitter in-between columns).

Flags: needinfo?(enndeakin)
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcc72374b769
add a previousColumn property for tree columns so that tree drag and drop will work without box objects, r=bzbarsky
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.