Closed
Bug 186229
Opened 22 years ago
Closed 20 years ago
BoxObject incorrect when scrollable ancestors
Categories
(Core :: Layout, defect, P4)
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: erik, Unassigned)
References
Details
(Keywords: testcase)
Attachments
(3 files)
The BoxObject for an element works fine in the simple scenario where only the
root element has a scrollable overflow. Whenever one or more ancestor elements
has been scrolled the returned box is incorrect and returns the value that would
have been correct when all the scrollable elements had scrollTop 0.
#e1, #e2 {
overflow: auto;
}
<element id="e1">
...
<element id="e2">
...
<element id="e3">...</element>
...
</element>
...
</element>
var y = document.getBoxObjectFor(document,getElementById("e3")).y
y is only correct if the elements e1 and e2 have not been scrolled.
Reporter | ||
Comment 1•22 years ago
|
||
Move the mouse around and note how the outline paints on top of the elements.
Then scroll any of the scrollable elements (you might need to resize your
widnow to see the scrollbars). Note now that the result from getBoxObjectFor
ais incorrect because the scrollTop of the ancestors are note taken into
account.
I'm sorry for the flickery nature of the test case but I did not want to waste
time on making it look nice.
Updated•22 years ago
|
Priority: -- → P4
Target Milestone: --- → Future
Comment 2•21 years ago
|
||
Comment 3•21 years ago
|
||
This patch adjusts the algorithm for calculating the boxObject screenX and
screenY so that it includes the scroll offsets for any scrolled views as it
works up the heirachy. This does not affect the boxObject values for the item
actually being scrolled, only it's children (and I think this is what it should
do, IMHO).
This is my first C++ patch, so any "you shouldn't do THAT!" comments are fine.
:)
Updated•21 years ago
|
Attachment #137734 -
Flags: review?(dbaron)
![]() |
||
Comment 4•21 years ago
|
||
Shouldn't this code just do what other similar callers do and walk the view
chain instead of the frame chain? For that matter, do we have enough people
needing this yet (I can think of at least two or three) to have a way to get
this info off an nsIView? Especially since people seem to screw it up 99% of
the time when they try to implement it?
![]() |
||
Comment 5•21 years ago
|
||
Actually, we could even put this on nsIFrame (have a ScreenRect() accessor or
something that properly walks the view tree to a widget, etc), since half the
magic is properly getting hold of a view...
Check out nsIView::GetNearestWidget in this patch (currently undergoing review :-) )
http://bugzilla.mozilla.org/attachment.cgi?id=135391&action=view
I think this is the right way to write this API. I want to replace
nsIFrame::GetOffsetFromView, nsIFrame::GetOriginToViewOffset and
nsIFrame::GetClosestView with a similar GetNearestView method. We should also
replace nsIFrame::GetWindow with an nsIFrame::GetNearestWidget and use it here.
James, do you want to have a go at reorganizing the functions like I said in my
last comment?
Comment 8•21 years ago
|
||
Robert, I could have a go, but I'm not too familiar with the frame/view/widget
stuff Mozilla uses, or C++ (not that that stopped me making a patch anyway :) ).
Comment on attachment 137734 [details] [diff] [review]
Fixed boxObject for boxes inside scrolled containers
Marking review- based on above comments from bz and roc.
Attachment #137734 -
Flags: review?(dbaron) → review-
![]() |
||
Updated•21 years ago
|
Keywords: helpwanted
Whiteboard: [good first bug]
![]() |
||
Comment 10•20 years ago
|
||
The function reorganization Robert wanted happened in bug 268576. This is fixed.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment 11•19 years ago
|
||
(In reply to comment #10)
> The function reorganization Robert wanted happened in bug 268576. This is
> fixed.
So this bug should be marked FIXED, right?
Comment 12•19 years ago
|
||
(In reply to comment #11)
> So this bug should be marked FIXED, right?
Ugh, ignore this comment please.
![]() |
||
Updated•19 years ago
|
Keywords: helpwanted
Whiteboard: [good first bug]
You need to log in
before you can comment on or make changes to this bug.
Description
•