Closed Bug 399961 Opened 12 years ago Closed 12 years ago

Expanding/collapsing container on tree with horizontal scrollbars buggy

Categories

(Core :: XUL, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: mimecuvalo, Assigned: neil)

References

Details

(Whiteboard: [has patch])

Attachments

(2 files, 1 obsolete file)

1.81 KB, application/vnd.mozilla.xul+xml
Details
903 bytes, patch
enndeakin
: review+
mtschrep
: approval1.9+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101104 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101104 Minefield/3.0a9pre

On a tree that has a primary="true" on its column and with horizontal scrollbars the + and - signs that expand/collapse the tree don't work consistently - sometimes they work and sometimes they don't.

This happens only if the tree has been scrolled horizontally over.

Reproducible: Always

Steps to Reproduce:
1. Open testcase file.
2. Scroll over a little bit
3. Try clicking on a '-' sign
Attached file Testcase
Blocks: 372206
So a workaround for this for now is you can double-click and that will work but that kind of sucks.

I'll give a shot at trying to write a patch for this bug myself.
Attached patch patch (obsolete) — Splinter Review
Attachment #287799 - Flags: review?(sspitzer)
Whiteboard: [has patch]
Actually, could this be bigger underlying problem?  B/c wouldn't you have to change everywhere getCellAt is called?
http://lxr.mozilla.org/mozilla/search?string=getcellat
Depends on: 212789
Is this perhaps a bug in getCellAt? For instance, if you have several columns, and you scroll some out of view, does getCellAt return the incorrect column?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to comment #5)
> Is this perhaps a bug in getCellAt? For instance, if you have several columns,
> and you scroll some out of view, does getCellAt return the incorrect column?
> 

Yeah, that's my suspicion.  Not sure, but this bug might be related as well: bug 391574 (which is related to bug 392216)
OK, so I've had a look at nsTreeBodyFrame::GetCellAt and it appears to be clipping the cell before hit-testing it. This has obvious repercussions in the case of left clipping, as it will hit-test as if the cell was wholly visible, but it also affects cells partially clipped on the right, as the border and padding will count against the right edge of the tree rather than the cell.
Attached patch Proposed patchSplinter Review
Removing the clipping seems to fix this.
Assignee: Jan.Varga → neil
Attachment #287799 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #287883 - Flags: superreview?(roc)
Attachment #287883 - Flags: review?(enndeakin)
Attachment #287799 - Flags: review?(sspitzer)
Attachment #287883 - Flags: superreview?(roc) → superreview+
I mentioned bug 391574 earlier and that is has the same issue basically except in a different function.  The function startEditing in tree.xml calls the function GetCoordsForCellItem in nsTreeBodyFrame.cpp which returns an incorrect x position (and actually sometimes an incorrect y position as per bug 392216).

Would the patch for that fix be similar to this one?  Seems like we can kill two birds with one stone here.
(In reply to comment #9)
>Would the patch for that fix be similar to this one?
I don't think so. My best guess for that bug is that the code is simply not taking horizontal scrolling into account. This bug is about code that's trying to take it into account, however it's getting it wrong.
Attachment #287883 - Flags: review?(enndeakin) → review+
Comment on attachment 287883 [details] [diff] [review]
Proposed patch

Fixes a bug in the implementation of a new feature.
Attachment #287883 - Flags: approval1.9?
Attachment #287883 - Flags: approval1.9? → approval1.9+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: xptoolkit.trees → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.