Open
Bug 501421
Opened 16 years ago
Updated 2 years ago
Implement getIntersectionList(), getEnclosureList()
Categories
(Core :: SVG, enhancement)
Core
SVG
Tracking
()
NEW
People
(Reporter: codedread, Unassigned)
References
(Blocks 1 open bug, )
Details
(Keywords: dev-doc-needed)
Attachments
(2 files, 4 obsolete files)
(deleted),
patch
|
jwatt
:
feedback-
|
Details | Diff | Splinter Review |
(deleted),
image/svg+xml
|
Details |
As mentioned here: http://www.mozilla.org/projects/svg/status.html please support the following methods on the <svg> DOM element:
getIntersectionList()
getEnclosureList()
checkIntersection()
checkClosure()
Spec details here: http://www.w3.org/TR/SVG11/struct.html#InterfaceSVGSVGElement
Opera supports these methods.
I have attached a very basic test case that covers getIntersectionList and getEnclosureList.
Comment 1•15 years ago
|
||
Reading that that Firefox 3.5- didn't support these methods [1], I got the impression that this was working post 3.5, and quickly launched my nightly build (3.6a [2])... But apparently not yet. :-|
[1] http://code.google.com/p/doctype/wiki/SVGSVGElement
[2] Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090724 Minefield/3.6a1pre (.NET CLR 3.5.30729)
Comment 2•15 years ago
|
||
Shouldn't this bug be for all platforms?
Comment 3•15 years ago
|
||
(In reply to comment #2)
> Shouldn't this bug be for all platforms?
Yup. :-)
OS: Mac OS X → All
Hardware: x86 → All
Updated•14 years ago
|
Blocks: ietestcenter
Comment 5•14 years ago
|
||
This is a testcase in which you move a rectangle by moving the mouse cursor. Fill color indicates whether the rect is enclosed by the box (another svg:rect). Stroke indicates whether the rect intersects the box.
Updated•14 years ago
|
Assignee: nobody → juca
Comment 6•14 years ago
|
||
This is work in progress. This patch works partially. With this patch the test case (475682) passes. But this patch is still incomplete. It is my first attempt at solving a non-trivial bug, so I'd like to have feedback on it. I am not sure I am doing things the best way.
some questions:
1) referenceElement is the root of a subtree for which every node should have its bbox checked against the provided rect for enclosure or intersection, right?
2) what is the precise definition of intersetion? I guess that an element that is intersecting rect must not be enclosed by rect at the same time, but that is not clear in the SVG spec.
3) when referenceElement is nsnull we should use the svg:svg element as root of the subtree to be searched for enclosure/intersections?
Please, give me some feedback on overall code quality / coding style.
Attachment #475692 -
Flags: review?(dholbert)
Reporter | ||
Comment 7•14 years ago
|
||
It's great that you're working on this - these methods are invaluable, in my opinion (equivalent of getElementsByClassName() etc for SVG).
For 1), please follow up with this issue I raised last year: http://www.w3.org/Graphics/SVG/WG/track/actions/2695 it looks like the spec was resolved and tests added. Unfortunately I don't remember how it was resolved :/
Reporter | ||
Comment 8•14 years ago
|
||
Here's the thread I started on www-svg: http://lists.w3.org/Archives/Public/www-svg/2009Nov/0015.html
Comment 9•14 years ago
|
||
Comment on attachment 475692 [details] [diff] [review]
wip. Need some advice please
I'm not familiar with the APIs being implemented in this bug, so I'd defer to jwatt or longsonr on giving feedback on this. Also, I'm changing "review?" to "feedback?", since (as you said) this isn't done yet & you're just looking for feedback.
Since you asked, here are some coding-style nits:
>+void
>+nsSVGSVGElement::evaluateList(PRBool enclosure_test, nsIContent* node,
>+ nsBaseContentList* contentList,
>+ gfxRect* ref){
Mozilla coding style is to have function names be capitalized in C++, so "evaluateList" should be capitalized.
Also (though we don't follow this everywhere), function-arguments should generally be prefixed with an "a" and be camelCase, not underscore_separated. (e.g. "aEnclosureTest", rather than "enclosure_test")
And the 2nd & 3rd lines quoted above need two more spaces of indentation to line up.
>+ if (ref->Contains(nodeBBox)){
>+ contentList->AppendElement(node);
>+ } else {
>+ return;
>+ }
>+ nsCOMPtr<nsINode> _node = do_QueryInterface(node);
Rename "node" to |aContent| or |aElement| (since it's a nsIContent*), and rename "_node" to "node".
>+ for (i=0; i < length; i++){
Add a space on either side of the "=".
>+ nsBaseContentList* contentList = new nsBaseContentList();
>+ NS_ENSURE_TRUE(contentList, NS_ERROR_OUT_OF_MEMORY);
"new" never fails in Mozilla code now (or rather, it aborts instead of failing), so there's no need to check for out-of-memory.
>+ void evaluateList(PRBool enclosure_test, nsIContent* node, nsBaseContentList* contentList,
>+ gfxRect* ref);
>+
This line is too long -- please wrap to <80 characters. (also, the indentation is off on the second line)
Attachment #475692 -
Flags: review?(dholbert) → feedback?(jwatt)
Reporter | ||
Comment 10•14 years ago
|
||
1) And checking SVG 1.1 second edition: http://www.w3.org/TR/2010/WD-SVG11-20100622/struct.html#__svg__SVGSVGElement__getIntersectionListIt says that referenceElement: "If not null, then any intersected element that doesn't have the referenceElement as ancestor must not be included in the returned NodeList."2) My interpretation of "intersects" is in the mathematical sense: If an element's bbox overlaps in any way with the argument rectangle.3) I would agree with your interpretation, another way to interpret this is when referenceElement is null, all elements are checked (so start at <svg>).
Comment 11•14 years ago
|
||
hmmm... It seems that the implementation of these functions is much more complicated than I had thought! I was considering only intersections of bounding boxes. We are really talking about intersection of the verctors themselves? That is certainly more complicated...
Comment 12•14 years ago
|
||
(In reply to comment #9)
> Also (though we don't follow this everywhere), function-arguments should
> generally be prefixed with an "a" and be camelCase, not underscore_separated.
> (e.g. "aEnclosureTest", rather than "enclosure_test")
Does this rule apply also to _retval ? should it be aRetVal instead or is it a special case where we prefer _lowercase ?
Comment 13•14 years ago
|
||
Mm, good question - "_retval" should be all-lowercase, according to an MXR search I just did. I'm not sure why, but that's the convention. :)
Comment 14•14 years ago
|
||
This is an updated version of the previous patch.
It has some refactoring work and it is closer to the mozilla coding style.
I have added 2 auxiliary methods to check for enclosure and for intersection of an element (nsIContent*) with a given gfxRect. These methods are currently only checking for enclosure and intersection of the bounding box of the element. These must be improved to also consider the pointer-events criteria.
I need feedback on:
1) proper iteration of the subtree (I am not sure about the correctness of my implementation)
2) how to get a reference for <svg> root element (for the case when referenceElement is nsnull)
3) how to proceed to implement the proper pointer-events criteria
Attachment #475692 -
Attachment is obsolete: true
Attachment #475884 -
Flags: feedback?(jwatt)
Attachment #475692 -
Flags: feedback?(jwatt)
Reporter | ||
Comment 15•14 years ago
|
||
I wrote a simple test for Opera this morning and they don't use bounding boxes for getIntersectionList()
Comment 16•13 years ago
|
||
Any chance this bug could get some love? Been open for a couple of years now.
I agree with Jeff's testing as far as Opera is concerned. Opera's implementation could thus be used as a reference.
Comment 17•12 years ago
|
||
Here is a more complete testcase. This checks complications such as non-rectilinear shapes, transforms, filters, stroke, pointer-events, marker, clipPath and mask.
The fill of any path elements that are included in the list returned by getIntersectionList will turn green.
I've not tested IE, but Webkit and Opera behave very differently.
Attachment #386062 -
Attachment is obsolete: true
Attachment #475682 -
Attachment is obsolete: true
Comment 18•12 years ago
|
||
Attachment #717967 -
Attachment is obsolete: true
Comment 19•12 years ago
|
||
Comment on attachment 475884 [details] [diff] [review]
wip. Works but only with simple boundingbox checks
Things that are unclear in the spec:
* what "rendered content" means
* what the effect of non-rectilinear shapes, transforms, filters, stroke,
pointer-events, marker, clipPath and mask should be
* what "Each candidate graphics element is to be considered a match only
if the same graphics element can be a target of pointer events as
defined in ‘pointer-events’ processing" means. (Opera seems to take this
to mean something like "points that render that would also react to
pointer events" contribute, whereas Webkit only seems to pay attention
to whether or not pointer-events="none".
I think at least these points need to be clarified before there's any point reviewing a patch, since the answers will greatly influence the patch.
Attachment #475884 -
Flags: feedback?(jwatt) → feedback-
Updated•12 years ago
|
Comment hidden (advocacy) |
Updated•10 years ago
|
Keywords: dev-doc-needed
Comment hidden (advocacy) |
Comment 22•9 years ago
|
||
This does not seems to be implemented in Nightly build 42.0a1
test FF (fails) Chrome IE (succeeded)
http://www.xn--dahlstrm-t4a.net/svg/interactivity/intersection/sandbox_hover.svg
Any confirmation ?
Comment 23•9 years ago
|
||
Test case for these and the other related methods (elementFromPoint, elementsFromPoint, etc)
http://jsfiddle.net/y0mthrd5/2/
FF is the last major browser not to have implemented these.
Comment 24•9 years ago
|
||
Although Chrome and Safari support getIntersectionList(), their implementation has a bug which severely limits the usefulness of the API:
- http://crbug.com/370012
- https://bugs.webkit.org/show_bug.cgi?id=81137
Comment hidden (advocacy) |
Updated•9 years ago
|
Blocks: svg-enhance
Comment hidden (obsolete) |
Comment 27•9 years ago
|
||
It seems we're waiting for answers to comment 19. Do you have them?
Flags: needinfo?(dmu)
Comment hidden (advocacy) |
Comment 30•8 years ago
|
||
(In reply to Robert Longson from comment #27)
> It seems we're waiting for answers to comment 19. Do you have them?
The SVG1.1 2e spec (16 August 2011) seems abundantly clear on the behavior, https://www.w3.org/TR/SVG11/struct.html#__svg__SVGSVGElement__getIntersectionList reads in full:
Returns the list of graphics elements whose rendered content intersects the supplied rectangle. Each candidate graphics element is to be considered a match only if the same graphics element can be a target of pointer events as defined in ‘pointer-events’ processing.
Put another way, you're given a rectangle and an element in question. If the element were "frontmost", could an anywhere within said rectangle result in a pointer event?
You must already have that logic for determining whether an element is under a given point for interaction purposes (clicks, hovers, etc.), right? Whatever "rendered content" and "non-rectilinear shapes, transforms, filters, stroke, pointer-events, marker, clipPath and mask" rules that apply *there* are clearly intended to apply *here*.
Comment 31•8 years ago
|
||
Sorry, bad editing on my key point:
If the element were "frontmost", could an _interaction_ anywhere within said rectangle result in a pointer event?
Rephrasing for mouse-only: could you click somewhere inside the given rect and hit the element in question, assuming nothing else in the way? If so, it's part of the list.
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Comment hidden (advocacy) |
Updated•7 years ago
|
Assignee: juca → nobody
Comment 36•7 years ago
|
||
There's one reported website that does not work in Firefox due to getIntersectionList().
More info: https://github.com/webcompat/web-bugs/issues/16558 (Please add this to the seeAlso list)
Comment hidden (advocacy) |
Comment 38•2 years ago
|
||
Clear a needinfo that is pending on an inactive user.
Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE
.
For more information, please visit auto_nag documentation.
Flags: needinfo?(juca)
Updated•2 years ago
|
Severity: normal → S3
Comment 39•2 years ago
|
||
The severity field for this bug is relatively low, S3. However, the bug has 24 votes.
:jwatt, could you consider increasing the bug severity?
For more information, please visit auto_nag documentation.
Flags: needinfo?(jwatt)
Comment 40•2 years ago
|
||
The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.
Flags: needinfo?(jwatt)
Updated•2 years ago
|
Severity: S3 → --
Type: defect → enhancement
You need to log in
before you can comment on or make changes to this bug.
Description
•