Bug 45467 - REGRESSION(r66731): pointer-events are broken in some cases
Summary: REGRESSION(r66731): pointer-events are broken in some cases
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dirk Schulze
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-09 10:12 PDT by Simon Fraser (smfr)
Modified: 2012-04-26 08:46 PDT (History)
10 users (show)

See Also:


Attachments
Testcase (1.18 KB, application/xhtml+xml)
2010-09-09 10:12 PDT, Simon Fraser (smfr)
no flags Details
Fast patch for SVG and pointer-events: none problem (3.65 KB, patch)
2010-12-02 08:05 PST, Victor Krapivin
no flags Details | Formatted Diff | Diff
script file (13.54 KB, text/x-js)
2011-05-20 17:00 PDT, Kevin Ar18
no flags Details
Example of SVG-HTML integration (5.60 KB, application/xhtml+xml)
2011-05-20 17:06 PDT, Kevin Ar18
no flags Details
Some SVG-HTML integration testcases (5.57 KB, application/xhtml+xml)
2011-05-20 17:17 PDT, Kevin Ar18
no flags Details
Updated SVG-HTML integration testcases (5.59 KB, application/xhtml+xml)
2011-05-20 17:21 PDT, Kevin Ar18
no flags Details
Patch (7.21 KB, patch)
2011-05-21 14:08 PDT, Dirk Schulze
no flags Details | Formatted Diff | Diff
2x Testcases to prove whether this bug is fixed or not (2.07 KB, application/xhtml+xml)
2011-05-21 18:58 PDT, Kevin Ar18
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2010-09-09 10:12:30 PDT
Created attachment 67050 [details]
Testcase

r66731 broke some pointer-event behavior; see attached testcase, and http://lists.w3.org/Archives/Public/public-fx/2010JulSep/0084.html
Comment 1 Kevin Ar18 2010-09-09 13:37:57 PDT
If it helps

I am using Windows

Bug present in:
r67080, r66933, r66820
Not present in 
r63305, r64732
Comment 2 Nikolas Zimmermann 2010-09-10 00:08:41 PDT
(In reply to comment #0)
> Created an attachment (id=67050) [details]
> Testcase
> 
> r66731 broke some pointer-event behavior; see attached testcase, and http://lists.w3.org/Archives/Public/public-fx/2010JulSep/0084.html

There's already another bug report regarding pointer-event behaviour on the "SVG Atlas map" example.
The new code doesn't respect pointer-events="none" on the outermost <svg>. Should be trivial to fix.
Comment 3 Victor Krapivin 2010-12-02 08:05:04 PST
Created attachment 75379 [details]
Fast patch for SVG and pointer-events: none problem

Problem: CSS style named pointer-events. Once it set to none, certain element should not be played in clicks.

Everything works, except latest changes in SVG. So, this fix is simple, as follows:

1. We have decomposition of pointer-events style to couple of flags. None means all flags false, so the condition 

    isVisible || !requireVisible

becomes true for case pointer-events: none. The fix is pretty simple - filter this case separately prior to flags analysis.

2. For background hit testing in SVG container, the pointer-events ain't played. Added simple check.

3. Added the same check for RenderForeignObject, as it had no analysis of pointer-events as well (and RenderBlock::nodeAtPoint called).
Comment 4 Victor Krapivin 2010-12-02 08:06:49 PST
The patch is tested at MacOSX with Chrome as well.
Comment 5 Nikolas Zimmermann 2010-12-02 08:33:20 PST
Comment on attachment 75379 [details]
Fast patch for SVG and pointer-events: none problem

View in context: https://bugs.webkit.org/attachment.cgi?id=75379&action=review

I'm not yet sure if this is correct, I'll have to check PointerEventsHitRules first, I'll come back later to your patch!
Thanks for the attempt! I'll see whether there's a less intrusive approach.

> WebKit/WebCore/rendering/RenderForeignObject.cpp:154
> +    if (style()->pointerEvents() == PE_NONE) return false;

The return statment needs to be placed in a seperated line.

> WebKit/WebCore/rendering/RenderSVGContainer.cpp:166
> +    if (style()->pointerEvents() == PE_NONE) return false;

Ditto.

> WebKit/WebCore/rendering/RenderSVGImage.cpp:153
> +    if (style()->pointerEvents() == PE_NONE) return false;

Ditto.

> WebKit/WebCore/rendering/RenderSVGRoot.cpp:334
> +    if (style()->pointerEvents() == PE_NONE) return false;

Ditto.

> WebKit/WebCore/rendering/svg/RenderSVGPath.cpp:262
> +    if (style()->pointerEvents() == PE_NONE) return false;

Ditto.

> WebKit/WebCore/rendering/svg/RenderSVGPath.cpp:262
> +    if (style()->pointerEvents() == PE_NONE) return false;

Ditto.
Comment 6 Bennett Rogers 2011-02-02 00:54:28 PST
Hello,

I was wondering if any progress had been made on the inclusion of this patch (or another) to fix this bug?  I'm depending on SVG and pointer-events, and while it appears that Safari is working properly, Chrome still is not.  I'm not sure if perhaps some other fix had been submitted that just hasn't made its way to Chromium, so I'm trying to track down the last known state of this issue.

Thanks!

(In reply to comment #5)
> (From update of attachment 75379 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=75379&action=review
> 
> I'm not yet sure if this is correct, I'll have to check PointerEventsHitRules first, I'll come back later to your patch!
> Thanks for the attempt! I'll see whether there's a less intrusive approach.
> 
> > WebKit/WebCore/rendering/RenderForeignObject.cpp:154
> > +    if (style()->pointerEvents() == PE_NONE) return false;
> 
> The return statment needs to be placed in a seperated line.
> 
> > WebKit/WebCore/rendering/RenderSVGContainer.cpp:166
> > +    if (style()->pointerEvents() == PE_NONE) return false;
> 
> Ditto.
> 
> > WebKit/WebCore/rendering/RenderSVGImage.cpp:153
> > +    if (style()->pointerEvents() == PE_NONE) return false;
> 
> Ditto.
> 
> > WebKit/WebCore/rendering/RenderSVGRoot.cpp:334
> > +    if (style()->pointerEvents() == PE_NONE) return false;
> 
> Ditto.
> 
> > WebKit/WebCore/rendering/svg/RenderSVGPath.cpp:262
> > +    if (style()->pointerEvents() == PE_NONE) return false;
> 
> Ditto.
> 
> > WebKit/WebCore/rendering/svg/RenderSVGPath.cpp:262
> > +    if (style()->pointerEvents() == PE_NONE) return false;
> 
> Ditto.
Comment 7 Dimitri Glazkov (Google) 2011-05-19 16:36:03 PDT
Sounds like this is going to break in Safari once it ships, since
Comment 8 Dimitri Glazkov (Google) 2011-05-19 16:36:17 PDT
... it's still not fixed.
Comment 9 Kevin Ar18 2011-05-20 16:59:34 PDT
Sounds like this is going to break in Safari once it ships, ...


That would be a shame.  This bug is very important to the future of SVG: it's practically key to using SVG in certain useful ways.

I'm gonna try to upload some files related to this (not exactly testcases for this particular bug), but should demonstrate why this may be more important than one might think at first.
--------------------
On another note:

I don't know much about Safari release cycles, but does this mean we could end up with a browser with this bug in it installed on people's systems for years to come (like the IE6 problem)?  If so, I would please ask that someone consider marking this bug as blocking for the next release as it is absolutely key to integrating SVG with HTML.  If we end up with many computers/devices that don't update and with this feature broken, it could be a problem for using SVG on websites.  On the other hand, if Safari auto-updates on all systems, then I guess there's not a big need for a change in priority.
Comment 10 Kevin Ar18 2011-05-20 17:00:55 PDT
Created attachment 94293 [details]
script file
Comment 11 Kevin Ar18 2011-05-20 17:06:41 PDT
Created attachment 94294 [details]
Example of SVG-HTML integration

This has been discussed on the SVG working group mailing list...

Rationale - Why this is an important issue.

The ability to integrated SVG and HTML, presents some interesting possibilities for future websites. Consider the following: 

* Using SVG to circle text or html items on a wepage. 
* Drawing arrows from one part of the page to the other (but not block content underneath). 
* Having oddly shaped menus, borders, layers, etc.... 
* Adding a "hazy" or semi-tranparent layer over html object, yet still be able to interact with the html object underneath. 
* Creating a graphical object "under" the cursor that follows the mouse, but still lets you click on items underneath it. 

If I remember correctly, none of this SVG-HTML integration will be possible if this bug/regression remains in Safari.

The attachment is an example of some of these items:
Comment 12 Kevin Ar18 2011-05-20 17:17:03 PDT
Created attachment 94297 [details]
Some SVG-HTML integration testcases

These are some testcases I started to make when I brought up the topic on the W3 SVG mailing list.


Anyways, I am mostly going on what I remember about this issue from several months back when I was discussing it on the W3 SVG and CSS mailing lists ...  so I may have gotten a few things wrong now.

Based on my tests back then, I was able to get SVG-HTML integration described in my previous post working on all major browsers (not sure abut IE9).  Then webkit broke.  I don't know if there is a workaround for webkit or if this means webkit can no longer do the special SVG-HTML integration.
Comment 13 Kevin Ar18 2011-05-20 17:21:46 PDT
Created attachment 94299 [details]
Updated SVG-HTML integration testcases

sorry, forgot to link the script file to bugzilla...
Comment 14 Dirk Schulze 2011-05-21 05:44:04 PDT
Thanks for all the comments and test cases. I can confirm that r66731 is the root of the problem. Other than the comment mentioned, the new behavior is still not in SVG1.1SE, but it was planned to add it during SVG Open 2010. I'd like to keep it, since Firefox and Opera seem to do it as well. I have a fix for the test cases on this bug, still need to clean up my patch and create a test case.
Comment 15 Dirk Schulze 2011-05-21 05:57:36 PDT
(In reply to comment #3)
> Created an attachment (id=75379) [details]
> Fast patch for SVG and pointer-events: none problem

This patch breaks more tests than it fixes. 3 of 4 tests of Kevin were broken after applying the patch.
Comment 16 Dirk Schulze 2011-05-21 11:23:14 PDT
Some background information

The problem is not cause by a wrong handling of visibility or any other CSS property. The idea of r66731 is, that a pointer can set on a SVG container, even if the container itself is not drawable. This can make things possible like http://vis.uell.net/pyramids/12te/

If you enable "Change Age-Groups", you can move a slider up and down. This is implemented by pointer-events and in this example, the pointer needs to get caught by the svg root element. This is not possible according to the SVG specification right now. But there was a discussion at SVG Open to make it possible with a lot of positive feedback. Niko implemented this to match the behavior of Firefox and (if I remember correctly) Opera.

I checked the behavior of browsers on the attached test case, as well as the pyramid example and got interesting results.

Operas behavior is similar to the behavior on Safari 5. The SVG root element doesn't caught the events. And like on Safari 5 it is not possible to track the slider over the SVG element, if no visible element is underneath.
Firefox supports the attached test case, but it is still possible to move the slider, not sure how Firefox is doing it.
Internet Explorer 9 matches exactly the behavior of WebKit trunk. I guess they optimized their behavior for the pyramid example.

I did not manage to get the behavior of Firefox, the preferred behavior, yet. If we do not find a solution, I'd suggest to revert r66731.
Comment 17 Dirk Schulze 2011-05-21 14:08:28 PDT
Created attachment 94328 [details]
Patch
Comment 18 Kevin Ar18 2011-05-21 18:02:20 PDT
(In reply to comment #9)
> I don't know much about Safari release cycles, but does this mean we could end up with a browser with this bug in it installed on people's systems for years to come (like the IE6 problem)?  If so, I would please ask that someone consider marking this bug as blocking for the next release ....
I meant to say that more as a plea depending on how releases are done for webkit/Safari; I don't want to imply it was a demand or anything.  Sorry; I didn't word it too well. :)

(In reply to comment #16)
> But there was a discussion at SVG Open to make it possible with a lot of positive feedback. Niko implemented this to match the behavior of Firefox and (if I remember correctly) Opera.

Dirk, have you been keeping up with the discussion on HTML-SVG integration?  It seems like I should probably ask some questions about this on the mailing list to see what the current status is.

I should note that the so-called "integration testcases" should not be taken as a definitive description of how every browser should handle SVG-HTML integration.  These tests were made several months back when there were still a number of questions as to how the standards and browsers should handle this.  I really should get up to date ... maybe I could even update the file into a proper testcase then.

Still, all of this should not affect this specific bug in webkit (the broken pointer-events:none).

Summary: Don't use the "integration testcases" as a way to confirm whether this bug report has been fixed.  In fact, I will probably upload another basic testcases from the w3 mailing list.
Comment 19 Kevin Ar18 2011-05-21 18:58:20 PDT
Created attachment 94332 [details]
2x Testcases to prove whether this bug is fixed or not

As mentioned... here is a simplified testcase (actually 2 of them) that should show whether this bug has been fixed or not.

There are a few interesting things to note:

(1) These testcases prove that the current bug is not critical; webkit passes 1 of the 2 testcases, which means there is at least some way to get proper HTML-SVG integration even with this bug present.

Also, here's some interesting results from various browsers:

Firefox 4.0B12pre (2011-02-16) = passes both
Opera 11.10, Build 2092 = passes both
IE 9.0.8112.16421 = fails test 1: floating; passes test 2: inline
Safari 5.0.5 (7533.21.1) - r87019 = fails test 1: floating; passes test 2: inline
Safari 5.0.5 (7533.21.1) - r80213 = fails test 1: floating; passes test 2: inline


The good news is one way to implement HTML-SVG integration that will work in all browsers (even with the bug in webkit), by using inline SVG.

As a side note, I was rather disappointed that IE failed one of the tests.  Anyways, hope this helps.
Comment 20 Dirk Schulze 2011-05-21 22:07:10 PDT
(In reply to comment #19)
> Created an attachment (id=94332) [details]

> Safari 5.0.5 (7533.21.1) - r87019 = fails test 1: floating; passes test 2: inline
> Safari 5.0.5 (7533.21.1) - r80213 = fails test 1: floating; passes test 2: inline

WebKit passes both again with the patch.
Comment 21 Dirk Schulze 2011-05-21 23:58:37 PDT
Comment on attachment 94328 [details]
Patch

Clearing flags on attachment: 94328

Committed r87027: <http://trac.webkit.org/changeset/87027>
Comment 22 Dirk Schulze 2011-05-21 23:58:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 23 Ademar Reis 2011-05-23 07:48:22 PDT
Revision r87027 cherry-picked into qtwebkit-2.2 with commit 91c0616 <http://gitorious.org/webkit/qtwebkit/commit/91c0616>
Comment 24 Riley Dutton 2012-04-26 06:17:19 PDT
This bug seems to have regressed again. The last attached test case (https://bugs.webkit.org/attachment.cgi?id=94332) is failing for me on Safari Version 5.1.4 (7534.54.16). Passes on latest release versions of Chrome or Firefox.
Comment 25 Simon Fraser (smfr) 2012-04-26 08:46:47 PDT
Safari 5.1.4 is much older than trunk. Try a webkit nightly.