Re: [webkit-dev] mouseenter and mouseleave events

2013-04-23 Thread Maciej Stachowiak

On Apr 22, 2013, at 3:32 PM, Allan Sandfeld Jensen k...@carewolf.com wrote:

 On Wednesday 17 April 2013, Maciej Stachowiak wrote:
 
 Seems like a reasonable feature to add.
 
 Would you mind also reviewing the patch in question then or help me find a 
 reviewer with the right knowledge? Most of the patch is boiler plate work. 
 The 
 central part is how and when the events are dispatched and confined to 
 additional responsibility of updateHoverActiveState.

I took a quick look at it and concluded that it would require some study of 
existing code to be confident of its correctness. I encourage reviewers with 
knowledge of DOM or events to take a look, as I will likely not have a chance 
for at least a few days.

Regards,
Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] mouseenter and mouseleave events

2013-04-22 Thread Allan Sandfeld Jensen
On Wednesday 17 April 2013, Maciej Stachowiak wrote:
 
 Seems like a reasonable feature to add.
 
Would you mind also reviewing the patch in question then or help me find a 
reviewer with the right knowledge? Most of the patch is boiler plate work. The 
central part is how and when the events are dispatched and confined to 
additional responsibility of updateHoverActiveState.

Regards
`Allan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] mouseenter and mouseleave events

2013-04-17 Thread Allan Sandfeld Jensen
On Tuesday 16 April 2013, you wrote:
 Hi
 
 On Tue, Apr 16, 2013 at 5:08 AM, Allan Sandfeld Jensen 
k...@carewolf.comwrote:
  I have recently uploaded a new patch to
  https://bugs.webkit.org/show_bug.cgi?id=18930 to implement mouseenter and
  mouseleave events.
 
 This sounds nice, but shouldn't you finish the support for image-rendering
 first?
 You added the code but it is not enabled anywhere. There is not one test
 covering the feature.
 
That is because it is not meant to be enabled any time soon. The CSS4 
definitions are only there to guide our implementation so it behaves in a 
forward compatible way. The problem is we enabled a small part of image-
rendering back when it was a proposal for CSS3 (and because it is part of SVG 
CSS). The standardization effort is in CSS4 and will probably take a few years 
before it is ready to be enabled on any ports.

Testing that animations are now in high quality when using the SVG CSS image-
rendering: optimizeQuality, is unfortunately not possible with our existing 
layout tests. I could add a manual test though, if you think that is better. I 
also have a patch to add the rest of the ifdefs for the feature so the related 
code is more easy to localize, but I don't like adding the feature to the 
build systems since it shouldn't really be enabled.

Not that I am going to defend the CSS4 parts being in the tree that hard. Now 
that I revisit the idea, I am less sure it was a good idea. It just seemed 
better than correcting an unstandardised feature without a guideline.

Best regards.
`Allan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] mouseenter and mouseleave events

2013-04-17 Thread Benjamin Poulain
On Wed, Apr 17, 2013 at 12:46 AM, Allan Sandfeld Jensen 
k...@carewolf.comwrote:

 That is because it is not meant to be enabled any time soon. The CSS4
 definitions are only there to guide our implementation so it behaves in a
 forward compatible way. The problem is we enabled a small part of image-
 rendering back when it was a proposal for CSS3 (and because it is part of
 SVG
 CSS). The standardization effort is in CSS4 and will probably take a few
 years
 before it is ready to be enabled on any ports.


If there is no plan to enable this anywhere for years. This should not be
in the source today.


 Testing that animations are now in high quality when using the SVG CSS
 image-
 rendering: optimizeQuality, is unfortunately not possible with our existing
 layout tests. I could add a manual test though, if you think that is
 better. I
 also have a patch to add the rest of the ifdefs for the feature so the
 related
 code is more easy to localize, but I don't like adding the feature to the
 build systems since it shouldn't really be enabled.


There should be at a minimum test for the parsing of the values.

Regarding testing the outcome itself...if the current infrastructure is
inadequate, you should look what can be done to fix that. Creating test
coverage is part of adding a feature, you should not dump code somewhere
and hope someone else will pick it up and ship it.

I would prefer you finish the feature before moving on the next one.

Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] mouseenter and mouseleave events

2013-04-17 Thread Allan Sandfeld Jensen
On Wednesday 17 April 2013, Benjamin Poulain wrote:
 On Wed, Apr 17, 2013 at 12:46 AM, Allan Sandfeld Jensen 
k...@carewolf.comwrote:
  That is because it is not meant to be enabled any time soon. The CSS4
  definitions are only there to guide our implementation so it behaves in a
  forward compatible way. The problem is we enabled a small part of image-
  rendering back when it was a proposal for CSS3 (and because it is part of
  SVG
  CSS). The standardization effort is in CSS4 and will probably take a few
  years
  before it is ready to be enabled on any ports.
 
 If there is no plan to enable this anywhere for years. This should not be
 in the source today.
 
I can understand that. I will see how to best clean it up, or possibly roll it 
back.

 
 I would prefer you finish the feature before moving on the next one.
 
When you are not working for Apple, it takes months to get a review of a 
patch. Just the former mouseenter/mouseleave event patch had been sitting in 
the review queue since September. If I worked on one thing at a time I would 
have months at a time were I would be sitting idle. Sorry I can't do that.

 `Allan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


[webkit-dev] mouseenter and mouseleave events

2013-04-16 Thread Allan Sandfeld Jensen
Hi webkit-dev

I have recently uploaded a new patch to 
https://bugs.webkit.org/show_bug.cgi?id=18930 to implement mouseenter and 
mouseleave events.

These events are part of DOM3 
http://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevents and supported by 
MSIE, Mozilla and pre-blink Opera. They are also 
listed as issues jQuery needs to work around in WebKit 
https://bugs.webkit.org/show_bug.cgi?id=110007

Previously they have not been implemented because they were not considered 
important, and could cause performance regression by issuing more events on 
every mouse-move that may not even be used by many websites.

After I refactored hit-testing in the fall and especially moved the update of 
hover/active state out of hit-testing, it is now possible to dispatch these 
events where the hover state is updated, which makes the logic of dispatching 
mouseenter and mouseleave events correctly simple. It is also possible to only 
do one check for possible capturing listeners before issuing any mouseenter or 
mouseleave events for a mousemove.

This is what the new patch does. It does incur an additional overhead by 
checking for the existence of capturing listeners, but the check is of lower 
magnitude than existing iterations over the tree, and should keep performance 
comparable, and is in all circumstances faster than issuing new events that 
fall for deaf ears.

Are there more objections for the support of mouseenter and mouseleave events 
in WebKit?

Best Regards
`Allan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] mouseenter and mouseleave events

2013-04-16 Thread Benjamin Poulain
Hi

On Tue, Apr 16, 2013 at 5:08 AM, Allan Sandfeld Jensen k...@carewolf.comwrote:

 I have recently uploaded a new patch to
 https://bugs.webkit.org/show_bug.cgi?id=18930 to implement mouseenter and
 mouseleave events.


This sounds nice, but shouldn't you finish the support for image-rendering
first?
You added the code but it is not enabled anywhere. There is not one test
covering the feature.

Benjamin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] mouseenter and mouseleave events

2013-04-16 Thread Maciej Stachowiak

On Apr 16, 2013, at 5:08 AM, Allan Sandfeld Jensen k...@carewolf.com wrote:

 Hi webkit-dev
 
 I have recently uploaded a new patch to 
 https://bugs.webkit.org/show_bug.cgi?id=18930 to implement mouseenter and 
 mouseleave events.
 
 These events are part of DOM3 
 http://www.w3.org/TR/DOM-Level-3-Events/#events-mouseevents and supported by 
 MSIE, Mozilla and pre-blink Opera. They are also 
 listed as issues jQuery needs to work around in WebKit 
 https://bugs.webkit.org/show_bug.cgi?id=110007
 
 Previously they have not been implemented because they were not considered 
 important, and could cause performance regression by issuing more events on 
 every mouse-move that may not even be used by many websites.
 
 After I refactored hit-testing in the fall and especially moved the update of 
 hover/active state out of hit-testing, it is now possible to dispatch these 
 events where the hover state is updated, which makes the logic of dispatching 
 mouseenter and mouseleave events correctly simple. It is also possible to 
 only 
 do one check for possible capturing listeners before issuing any mouseenter 
 or 
 mouseleave events for a mousemove.
 
 This is what the new patch does. It does incur an additional overhead by 
 checking for the existence of capturing listeners, but the check is of lower 
 magnitude than existing iterations over the tree, and should keep performance 
 comparable, and is in all circumstances faster than issuing new events that 
 fall for deaf ears.
 
 Are there more objections for the support of mouseenter and mouseleave events 
 in WebKit?

Seems like a reasonable feature to add.

 - Maciej

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev