Closed Bug 388927 Opened 17 years ago Closed 17 years ago

GetChildAtPoint() fails for scrolled content

Categories

(Core :: Disability Access APIs, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

(Keywords: access)

Attachments

(3 files, 2 obsolete files)

Attached file testcase (deleted) —
make window size to see the half of table only then scroll to second column and click it. GetChildAtPoint() reutrn document accessible, but it should retunr table accessible. The problem is GetBounds() return negative values.
btw, it works fine on fx2
Robert, if the frame is scrolled (is bigger than browser's window) then what will nsIFrame::getScreenRectExternal() return? For example, frame size -----------------------| | | | | | window area | |----------------------|
It will always return the full size. GetScreenRectExternal doesn't do any clipping.
(In reply to comment #3) > It will always return the full size. GetScreenRectExternal doesn't do any > clipping. > Ok, I get it for width/height. But what about x/y if left part of the frame is invisible (scrolled out). I think left-top edge of window area (visible area) should be (0, 0) so what is x/y?
If the frame is scrolled up or left beyond the top or left edge of the window, its screen rect x or y can be negative; again, we're not clipping it to the window edges.
Ok, how can I get rect of visible area only in coords relative screen?
Assignee: aaronleventhal → surkov.alexander
I'm not exactly sure what GetChildAtPoint is supposed to do, but it should probably be using nsLayoutUtils::GetFrameForPoint in some way.
Attached patch patch (obsolete) (deleted) — Splinter Review
I follow to the next logic: find accessible inside direct children, otherwise return null. It fixed this bug. But unfortunately it doesn't fix bug 343904. I took idea of the patch from there. I noticed the xul trees should have own implementation of the method. I'm not sure do we have another accessibles like trees.
Attachment #278052 - Flags: review?(aaronleventhal)
Comment on attachment 278052 [details] [diff] [review] patch Evan, can you be the reviewer?
Attachment #278052 - Flags: review?(aaronleventhal) → review?(Evan.Yan)
Attached patch patch2 (obsolete) (deleted) — Splinter Review
fix for tree columns
Attachment #278052 - Attachment is obsolete: true
Attachment #278121 - Flags: review?(Evan.Yan)
Attachment #278052 - Flags: review?(Evan.Yan)
Attachment #278121 - Flags: superreview?(roc)
Attachment #278121 - Flags: review?(roc)
er.. when I was changing the product field of this bug from "firefox" to "core", the flag "blocking-firefox3" was cleared automatically. I didn't find where I can reset it. Aaron, please reset it if needed. Sorry about that.
sorry, my latest comment was meant for bug 393094... I was a bonehead.
Comment on attachment 278121 [details] [diff] [review] patch2 >+ >+ return GetCachedTreeitemAccessible(row, column, aAccessible); >+} >+ Have you tested whether it works when tree view changed, like sorting by another column, moving columns? Currently we didn't update cache when tree view changed, refer to bug 368835 comment #28.
(In reply to comment #13) > (From update of attachment 278121 [details] [diff] [review]) > >+ > >+ return GetCachedTreeitemAccessible(row, column, aAccessible); > >+} > >+ > Have you tested whether it works when tree view changed, like sorting by > another column, moving columns? > Currently we didn't update cache when tree view changed, refer to bug 368835 > comment #28. > No, I didn't tested. But I think it shouldn't work like it doens't work now because now we getChildAtPoint uses bounds of accessible and since we do not update cache then we'll return bounds for obsolete accessibles.
I guess that depend on the |row| returned from mTree->GetCellAt(), is the row of _model_ or the row of _view_
(In reply to comment #15) > I guess that depend on the |row| returned from mTree->GetCellAt(), is the row > of _model_ or the row of _view_ > Evan, sorry, I'm not sure I got you. Can you rephrase it?
Attached file tree testcase (requires chrome privs) (deleted) —
Evan, sort and columndrag seems to work. I failed because treeitem accessibles are wrappers for methods of nsITreeView therefore our cache is always valid (though probably is bigger than treeitems count).
(In reply to comment #16) > Evan, sorry, I'm not sure I got you. Can you rephrase it? > Well, I was talking about MVC pattern, which was followed by xul tree implementation, I think. I talked about two kind of row index. One is the row of view, i.e. which line it shows on screen; The other is the row of model, i.e. the index it stored in its data structure. When the tree view changed, the row of view changed, but the row of model keeps the same. So, in your patch, if mTree->GetCellAt() returned the row of view, then we use it to call GetCachedTreeitemAccessible(), we probably can get the right accessible.
(In reply to comment #17) > Created an attachment (id=278522) [details] > tree testcase (requires chrome privs) > > Evan, sort and columndrag seems to work. Great! > I failed because treeitem accessibles > are wrappers for methods of nsITreeView therefore our cache is always valid > (though probably is bigger than treeitems count). > Yea, that's the point of my confusing comment ;) Sorry about my bad expression.
Attachment #278121 - Flags: review?(Evan.Yan) → review+
Status: NEW → ASSIGNED
Attached patch patch3 (deleted) — Splinter Review
logic to keep out of flow elements from bug 343904
Attachment #278121 - Attachment is obsolete: true
Attachment #278952 - Flags: review?(aaronleventhal)
Attachment #278121 - Flags: superreview?(roc)
Attachment #278121 - Flags: review?(roc)
Attachment #278952 - Flags: superreview?(roc)
Attachment #278952 - Flags: review?(roc)
Attachment #278952 - Flags: review?(aaronleventhal) → review?(ginn.chen)
Does this fix the out of flow accessibles as well?
Attachment #278952 - Flags: review?(ginn.chen) → review?(Evan.Yan)
Comment on attachment 278952 [details] [diff] [review] patch3 looks good.
Attachment #278952 - Flags: review?(Evan.Yan) → review+
Attachment #278952 - Flags: superreview?(roc)
Attachment #278952 - Flags: superreview+
Attachment #278952 - Flags: review?(roc)
Attachment #278952 - Flags: review+
(In reply to comment #21) > Does this fix the out of flow accessibles as well? > Yes, I tested for absolute position.
Attachment #278952 - Flags: approval1.9?
> Yes, I tested for absolute position. Do you mean it fixes bug 343904?
(In reply to comment #25) > > Yes, I tested for absolute position. > > Do you mean it fixes bug 343904? > Yep but I should set STATE_FLOATING additionally I guess. Are there another problems with states for out of flow objects?
No other problems with states.
Attachment #278952 - Flags: approval1.9? → approval1.9+
checked in
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Just wondering, could this fix be causing bug 471493? (mutually recursive functions causing a stack overflow)
(In reply to comment #29) > Just wondering, could this fix be causing bug 471493? (mutually recursive > functions causing a stack overflow) No, I think it's bug 455442.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: