BoxObject incorrect when scrollable ancestors




16 years ago
13 years ago


(Reporter: erik, Unassigned)



Windows XP

Firefox Tracking Flags

(Not tracked)



(3 attachments)



16 years ago
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>

var y = document.getBoxObjectFor(document,getElementById("e3")).y

y is only correct if the elements e1 and e2 have not been scrolled.

Comment 1

16 years ago
Created attachment 109807 [details]
Paints an outline on top of the element given the info from the BoxObject

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

I'm sorry for the flickery nature of the test case but I did not want to waste
time on making it look nice.


16 years ago
Keywords: testcase
Priority: -- → P4
Target Milestone: --- → Future

Comment 2

15 years ago
Created attachment 137733 [details]
Shows the values of various boxObject properties for nested scrolling container.

Comment 3

15 years ago
Created attachment 137734 [details] [diff] [review]
Fixed boxObject for boxes inside scrolled containers

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.


15 years ago
Attachment #137734 - Flags: review?(dbaron)
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?
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 :-) )

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

15 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-
Keywords: helpwanted
Whiteboard: [good first bug]
The function reorganization Robert wanted happened in bug 268576.  This is fixed.
Last Resolved: 14 years ago
Resolution: --- → FIXED
(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?
(In reply to comment #11)
> So this bug should be marked FIXED, right?
Ugh, ignore this comment please. 

Keywords: helpwanted
Whiteboard: [good first bug]
You need to log in before you can comment on or make changes to this bug.