116896 2013-05-28 15:22:07 -0700 ENABLE(PAN_SCROLLING) AutoscrollController::updateAutoscrollRenderer calls hitTestResultAtPoint with `true` for HitTestRequestType 2013-06-06 10:10:15 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck allan.jensen allan.jensen bweinstein commit-queue joepeck jturcotte oldest_to_newest 894566 0 joepeck 2013-05-28 15:22:07 -0700 This looks like stale code after hitTestResultAtPoint changed signatures and replaced the second boolean parameter with an optional bitfield parameter. AutoscrollController::updateAutoscrollRenderer passes bool true for a parameter that used to mean "allowsShadowDOM", but the parameter is now a bitfield of flags, one of which is AllowsShadowDOM. This could would need to be updated. Or, better yet, if no ports enable PAN_SCROLLING the feature could be removed. 894573 1 joepeck 2013-05-28 15:32:44 -0700 See bug 95720: <http://webkit.org/b/95720> Simplify hitTestResultAtPoint and nodesFromRect APIs 897423 2 allan.jensen 2013-06-05 08:00:48 -0700 Thanks, that looks like a bit of confusion between two different refactorings. 897424 3 203853 allan.jensen 2013-06-05 08:06:39 -0700 Created attachment 203853 Patch 897425 4 allan.jensen 2013-06-05 08:08:42 -0700 Note that allowsShadowDOM is now also implied, and you only need to add a flag to disallow it. 897507 5 203853 joepeck 2013-06-05 12:10:28 -0700 Comment on attachment 203853 Patch I think at the time this was written the signature of hitTestResultAtPoint was: HitTestResult hitTestResultAtPoint(const LayoutPoint&, bool allowShadowContent, bool ignoreClipping = false, HitTestScrollbars scrollbars = DontHitTestScrollbars, HitTestRequest::HitTestRequestType hitType = HitTestRequest::ReadOnly | HitTestRequest::Active, const LayoutSize& padding = LayoutSize()); Current trunk is: HitTestResult hitTestResultAtPoint(const LayoutPoint&, HitTestRequest::HitTestRequestType hitType = HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowShadowContent, const LayoutSize& padding = LayoutSize()); So I believe this code intends to have: HitTestRequest::ReadOnly | HitTestRequest::Active (+ allow shadow DOM which is implicit) But your change above ends up with just: HitTestRequest::ReadOnly (+ allow shadow DOM which is implicit) Does this also need to include HitTestRequest::Active? 897763 6 allan.jensen 2013-06-06 07:01:35 -0700 (In reply to comment #5) > (From update of attachment 203853 [details]) > I think at the time this was written the signature of hitTestResultAtPoint was: > > HitTestResult hitTestResultAtPoint(const LayoutPoint&, bool allowShadowContent, > bool ignoreClipping = false, HitTestScrollbars scrollbars = DontHitTestScrollbars, > HitTestRequest::HitTestRequestType hitType = HitTestRequest::ReadOnly | HitTestRequest::Active, > const LayoutSize& padding = LayoutSize()); > > Current trunk is: > > HitTestResult hitTestResultAtPoint(const LayoutPoint&, > HitTestRequest::HitTestRequestType hitType = HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::DisallowShadowContent, > const LayoutSize& padding = LayoutSize()); > > So I believe this code intends to have: > > HitTestRequest::ReadOnly | HitTestRequest::Active (+ allow shadow DOM which is implicit) > > But your change above ends up with just: > > HitTestRequest::ReadOnly (+ allow shadow DOM which is implicit) > > Does this also need to include HitTestRequest::Active? To be the same call yes, it shouldn't matter, but I can re-add it to keep the code equivalent. 897764 7 203932 allan.jensen 2013-06-06 07:04:04 -0700 Created attachment 203932 Patch 897829 8 203932 kling 2013-06-06 09:56:42 -0700 Comment on attachment 203932 Patch r=me 897838 9 203932 commit-queue 2013-06-06 10:10:12 -0700 Comment on attachment 203932 Patch Clearing flags on attachment: 203932 Committed r151281: <http://trac.webkit.org/changeset/151281> 897839 10 commit-queue 2013-06-06 10:10:15 -0700 All reviewed patches have been landed. Closing bug. 203853 2013-06-05 08:06:39 -0700 2013-06-06 07:03:58 -0700 Patch bug-116896-20130605170508.patch text/plain 1688 allan.jensen U3VidmVyc2lvbiBSZXZpc2lvbjogMTUxMjI0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDhhOWViZTYxYWM1NzNm ZDU0ODNlYzQ1ZTllMzRlZmQxZGJkMWEwMi4uMGYyYTBmNmMzOTQyNTU5Y2VjNGMyY2NlNWI0ZWI5 OGJmNmI3NDU2NiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTA2LTA1ICBBbGxh biBTYW5kZmVsZCBKZW5zZW4gIDxhbGxhbi5qZW5zZW5AZGlnaWEuY29tPgorCisgICAgICAgIEVO QUJMRShQQU5fU0NST0xMSU5HKSBBdXRvc2Nyb2xsQ29udHJvbGxlcjo6dXBkYXRlQXV0b3Njcm9s bFJlbmRlcmVyIGNhbGxzIGhpdFRlc3RSZXN1bHRBdFBvaW50IHdpdGggYHRydWVgIGZvciBIaXRU ZXN0UmVxdWVzdFR5cGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTExNjg5NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFVwZGF0ZSBoaXRUZXN0UmVzdWx0QXRQb2ludCBjYWxsIHRvIG5ldyBBUEkuCisKKyAg ICAgICAgKiBwYWdlL0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkF1dG9zY3JvbGxDb250cm9sbGVyOjp1cGRhdGVBdXRvc2Nyb2xsUmVuZGVyZXIpOgorCiAyMDEz LTA2LTA1ICBDaHJpc3RvcGhlIER1bWV6ICA8Y2guZHVtZXpAc2lzYS5zYW1zdW5nLmNvbT4KIAog ICAgICAgICBVbnJldmlld2VkIGJ1aWxkIGZpeCB3aGVuIENIQU5ORUxfTUVTU0FHSU5HIGlzIGRp c2FibGVkLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGFnZS9BdXRvc2Nyb2xsQ29udHJv bGxlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcApp bmRleCBhYjdjNDRlODVmMjk4NGIxZDY5MGZjY2ExMmU5YTcwMTVmN2VmMmU1Li4zYjdkZGZmZDJk MzE0YjM4NGFjMWNmN2VkZWMwNjNhYzcwNjhjNzQ5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9wYWdlL0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wYWdl L0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcApAQCAtMTMzLDcgKzEzMyw3IEBAIHZvaWQgQXV0b3Nj cm9sbENvbnRyb2xsZXI6OnVwZGF0ZUF1dG9zY3JvbGxSZW5kZXJlcigpCiAgICAgUmVuZGVyT2Jq ZWN0KiByZW5kZXJlciA9IG1fYXV0b3Njcm9sbFJlbmRlcmVyOwogCiAjaWYgRU5BQkxFKFBBTl9T Q1JPTExJTkcpCi0gICAgSGl0VGVzdFJlc3VsdCBoaXRUZXN0ID0gbV9hdXRvc2Nyb2xsUmVuZGVy ZXItPmZyYW1lKCktPmV2ZW50SGFuZGxlcigpLT5oaXRUZXN0UmVzdWx0QXRQb2ludChtX3BhblNj cm9sbFN0YXJ0UG9zLCB0cnVlKTsKKyAgICBIaXRUZXN0UmVzdWx0IGhpdFRlc3QgPSBtX2F1dG9z Y3JvbGxSZW5kZXJlci0+ZnJhbWUoKS0+ZXZlbnRIYW5kbGVyKCktPmhpdFRlc3RSZXN1bHRBdFBv aW50KG1fcGFuU2Nyb2xsU3RhcnRQb3MsIEhpdFRlc3RSZXF1ZXN0OjpSZWFkT25seSk7CiAKICAg ICBpZiAoTm9kZSogbm9kZUF0UG9pbnQgPSBoaXRUZXN0LmlubmVyTm9kZSgpKQogICAgICAgICBy ZW5kZXJlciA9IG5vZGVBdFBvaW50LT5yZW5kZXJlcigpOwo= 203932 2013-06-06 07:04:04 -0700 2013-06-06 10:10:11 -0700 Patch bug-116896-20130606160231.patch text/plain 1725 allan.jensen U3VidmVyc2lvbiBSZXZpc2lvbjogMTUxMjY5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzc2MTg1OTFlMDYzOGZl YTA1YzU3YjdiNzkwOTBiMGVhMmJkMzgwNy4uODQwZGQ2M2FmOGNkYWJiNmQ1ZmFkN2FmZmNlZDI2 ZDRhN2M2OWZlMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE1IEBACisyMDEzLTA2LTA2ICBBbGxh biBTYW5kZmVsZCBKZW5zZW4gIDxhbGxhbi5qZW5zZW5AZGlnaWEuY29tPgorCisgICAgICAgIEVO QUJMRShQQU5fU0NST0xMSU5HKSBBdXRvc2Nyb2xsQ29udHJvbGxlcjo6dXBkYXRlQXV0b3Njcm9s bFJlbmRlcmVyIGNhbGxzIGhpdFRlc3RSZXN1bHRBdFBvaW50IHdpdGggYHRydWVgIGZvciBIaXRU ZXN0UmVxdWVzdFR5cGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTExNjg5NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFVwZGF0ZSBoaXRUZXN0UmVzdWx0QXRQb2ludCBjYWxsIHRvIG5ldyBBUEkuCisKKyAg ICAgICAgKiBwYWdlL0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OkF1dG9zY3JvbGxDb250cm9sbGVyOjp1cGRhdGVBdXRvc2Nyb2xsUmVuZGVyZXIpOgorCiAyMDEz LTA2LTA2ICBDaHJpc3RvcGhlIER1bWV6ICA8Y2guZHVtZXpAc2lzYS5zYW1zdW5nLmNvbT4KIAog ICAgICAgICBSZW1vdmUgdW51c2VkIEhhc2hWYWx1ZUZvckNsYXNzQW5kTmFtZSgpIGZyb20gSlND IGJpbmRpbmdzIGdlbmVyYXRvcgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGFnZS9BdXRv c2Nyb2xsQ29udHJvbGxlci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL0F1dG9zY3JvbGxDb250 cm9sbGVyLmNwcAppbmRleCBhYjdjNDRlODVmMjk4NGIxZDY5MGZjY2ExMmU5YTcwMTVmN2VmMmU1 Li43MjU3ZWQ0OWI0OWQwOWRiMDc3ZjVmZjQ1NGM4ZGZkMWMxMGM1ZmI0IDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViQ29yZS9wYWdlL0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcAorKysgYi9Tb3VyY2Uv V2ViQ29yZS9wYWdlL0F1dG9zY3JvbGxDb250cm9sbGVyLmNwcApAQCAtMTMzLDcgKzEzMyw3IEBA IHZvaWQgQXV0b3Njcm9sbENvbnRyb2xsZXI6OnVwZGF0ZUF1dG9zY3JvbGxSZW5kZXJlcigpCiAg ICAgUmVuZGVyT2JqZWN0KiByZW5kZXJlciA9IG1fYXV0b3Njcm9sbFJlbmRlcmVyOwogCiAjaWYg RU5BQkxFKFBBTl9TQ1JPTExJTkcpCi0gICAgSGl0VGVzdFJlc3VsdCBoaXRUZXN0ID0gbV9hdXRv c2Nyb2xsUmVuZGVyZXItPmZyYW1lKCktPmV2ZW50SGFuZGxlcigpLT5oaXRUZXN0UmVzdWx0QXRQ b2ludChtX3BhblNjcm9sbFN0YXJ0UG9zLCB0cnVlKTsKKyAgICBIaXRUZXN0UmVzdWx0IGhpdFRl c3QgPSBtX2F1dG9zY3JvbGxSZW5kZXJlci0+ZnJhbWUoKS0+ZXZlbnRIYW5kbGVyKCktPmhpdFRl c3RSZXN1bHRBdFBvaW50KG1fcGFuU2Nyb2xsU3RhcnRQb3MsIEhpdFRlc3RSZXF1ZXN0OjpSZWFk T25seSB8IEhpdFRlc3RSZXF1ZXN0OjpBY3RpdmUpOwogCiAgICAgaWYgKE5vZGUqIG5vZGVBdFBv aW50ID0gaGl0VGVzdC5pbm5lck5vZGUoKSkKICAgICAgICAgcmVuZGVyZXIgPSBub2RlQXRQb2lu dC0+cmVuZGVyZXIoKTsK