149835 2015-10-05 23:09:19 -0700 Slider knobs should scale when rendering while zoomed 2015-10-06 10:28:31 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Mac Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 wenson_hsieh webkit-unassigned commit-queue esprehn+autocc glenn kondapallykalyan webkit-bug-importer oldest_to_newest 1130890 0 wenson_hsieh 2015-10-05 23:09:19 -0700 The knobs of sliders (<input>s of type range) should scale when pinching to zoom. 1130947 1 262514 wenson_hsieh 2015-10-06 07:15:14 -0700 Created attachment 262514 Patch 1130983 2 262514 darin 2015-10-06 09:26:06 -0700 Comment on attachment 262514 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262514&action=review Is there a good way to make a regression test for this? I don’t like making this fix without a test. > Source/WebCore/platform/mac/ThemeMac.mm:688 > + std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::createCompatibleBuffer(rect.size() + 2 * FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth), deviceScaleFactor, ColorSpaceSRGB, context, false); I suggest we use auto here instead of explicitly listing the type. > Source/WebCore/platform/mac/ThemeMac.mm:697 > + needsRepaint = drawCellOrFocusRingIntoRectWithView(cell, NSRect(rect), view, drawButtonCell, drawFocusRing); Why the syntax NSRect(rect) here? Won’t the code compile without the explicit conversation? 1131008 3 262514 wenson_hsieh 2015-10-06 10:11:40 -0700 Comment on attachment 262514 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=262514&action=review Thank you for the review! In regards to testing, there isn't currently a good way to test rendering of zoomed/scaled form controls. When getting other widgets to zoom a few weeks ago, I discussed ways to test this new behavior with Dan and Alexey, and we determined the only way to do this was to write pixel tests, but that since we don't run pixel tests the bots, writing new pixel tests wasn't that useful. We brainstormed a few ideas to address this issue and ended up filing https://bugs.webkit.org/show_bug.cgi?id=147193, but we'll need to have a broader conversation about ways to test rendering in WebKit beyond pixel tests. >> Source/WebCore/platform/mac/ThemeMac.mm:688 >> + std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::createCompatibleBuffer(rect.size() + 2 * FloatSize(buttonFocusRectOutlineWidth, buttonFocusRectOutlineWidth), deviceScaleFactor, ColorSpaceSRGB, context, false); > > I suggest we use auto here instead of explicitly listing the type. Got it -- changed. >> Source/WebCore/platform/mac/ThemeMac.mm:697 >> + needsRepaint = drawCellOrFocusRingIntoRectWithView(cell, NSRect(rect), view, drawButtonCell, drawFocusRing); > > Why the syntax NSRect(rect) here? Won’t the code compile without the explicit conversation? Unfortunately, without the explicit conversion, 32-bit builds won't compile. 1131015 4 wenson_hsieh 2015-10-06 10:28:31 -0700 Committed r190623: <http://trac.webkit.org/changeset/190623> 262514 2015-10-06 07:15:14 -0700 2015-10-06 09:26:06 -0700 Patch bug-149835-20151006071442.patch text/plain 5688 wenson_hsieh U3VidmVyc2lvbiBSZXZpc2lvbjogMTkwNDUwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZTE4MDc1YjI1MGM1MDQw NmMwYjVhYTMxNmQ5ODQ4YjAxYmRkYjI0Ni4uZDQ2NjVlMGFhMzc4OTAwNDZkOGI1ZTc5MWM3Yzg1 YmFhZDA2MjMxNyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDE1LTEwLTA1ICBXZW5z b24gSHNpZWggIDx3ZW5zb25faHNpZWhAYXBwbGUuY29tPgorCisgICAgICAgIFNsaWRlciBrbm9i cyBzaG91bGQgc2NhbGUgd2hlbiByZW5kZXJpbmcgd2hpbGUgem9vbWVkCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDk4MzUKKworICAgICAgICBSZXZp ZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBNYWtlIHNsaWRlciBrbm9icyBmb2xs b3cgc3VpdCB3aXRoIHRoZSByZXN0IG9mIHRoZSB1bnNjYWxlZCBmb3JtIGNvbnRyb2xzCisgICAg ICAgIGJ5IHJlbmRlcmluZyB0byBhbiBvZmZzY3JlZW4gYnVmZmVyIHdoZW4gdGhlIHBhZ2UgaXMg em9vbWVkIG9yIHNjYWxlZCBhbmQKKyAgICAgICAgdGhlbiByZW5kZXJpbmcgYSBzY2FsZWQgdmVy c2lvbiBvZiB0aGUgb2Zmc2NyZWVuIGJ1ZmZlciBvbnRvIHRoZSBwYWdlLgorCisgICAgICAgICog cGxhdGZvcm0vbWFjL1RoZW1lTWFjLm1tOgorICAgICAgICAoV2ViQ29yZTo6ZHJhd0NlbGxPckZv Y3VzUmluZ0ludG9SZWN0V2l0aFZpZXcpOiBIZWxwZXIgZnVuY3Rpb24gZm9yIGRyYXdpbmcKKyAg ICAgICAgICAgIGNlbGxzIGFuZC9vciBmb2N1cyByaW5ncy4KKyAgICAgICAgKFdlYkNvcmU6OlRo ZW1lTWFjOjpkcmF3Q2VsbE9yRm9jdXNSaW5nV2l0aFZpZXdJbnRvQ29udGV4dCk6IFJlZmFjdG9y ZWQgdG8KKyAgICAgICAgICAgIGhhbmRsZSBkcmF3aW5nIHNsaWRlciBrbm9icyBhcyB3ZWxsLgor ICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJUaGVtZU1hYy5tbToKKyAgICAgICAgKFdlYkNvcmU6 OlJlbmRlclRoZW1lTWFjOjpwYWludFNsaWRlclRodW1iKTogVXNlIHNjYWxlZCByZW5kZXJpbmcg d2hlbiBuZWNlc3NhcnkuCisKIDIwMTUtMTAtMDEgIERlYW4gSmFja3NvbiAgPGRpbm9AYXBwbGUu Y29tPgogCiAgICAgICAgIFJvbGxvdXQgcjE5MDQ0NiBmb3IgdGhlIG1vbWVudC4gSXQgYnJva2Ug dGhlIGJ1aWxkLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWFjL1RoZW1l TWFjLm1tIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWFjL1RoZW1lTWFjLm1tCmluZGV4IDlk NDYyNmFkZTYxNDQyMzQyYTdkM2ZiMGY5OGMxMzJiY2U1NzYyZDguLjZmNDQzOWM1NGExMTI3YzQ5 N2EzZjQzNDY3NTE1MjJiNzBiMmU0NmQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL21hYy9UaGVtZU1hYy5tbQorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9tYWMvVGhl bWVNYWMubW0KQEAgLTY2NCwyOSArNjY0LDM3IEBAIHZvaWQgVGhlbWVNYWM6OnNldEZvY3VzUmlu Z0NsaXBSZWN0KGNvbnN0IEZsb2F0UmVjdCYgcmVjdCkKIAogY29uc3QgZmxvYXQgYnV0dG9uRm9j dXNSZWN0T3V0bGluZVdpZHRoID0gMy4wZjsKIAotYm9vbCBUaGVtZU1hYzo6ZHJhd0NlbGxPckZv Y3VzUmluZ1dpdGhWaWV3SW50b0NvbnRleHQoTlNDZWxsKiBjZWxsLCBHcmFwaGljc0NvbnRleHQm IGNvbnRleHQsIGNvbnN0IEZsb2F0UmVjdCYgaW5mbGF0ZWRSZWN0LCBOU1ZpZXcqIHZpZXcsIGJv b2wgZHJhd0J1dHRvbkNlbGwsIGJvb2wgZHJhd0ZvY3VzUmluZywgYm9vbCB1c2VJbWFnZUJ1ZmZl ciwgZmxvYXQgZGV2aWNlU2NhbGVGYWN0b3IpCitzdGF0aWMgaW5saW5lIGJvb2wgZHJhd0NlbGxP ckZvY3VzUmluZ0ludG9SZWN0V2l0aFZpZXcoTlNDZWxsICpjZWxsLCBOU1JlY3QgcmVjdCwgTlNW aWV3ICp2aWV3LCBib29sIGRyYXdCdXR0b25DZWxsLCBib29sIGRyYXdGb2N1c1JpbmcpCit7Cisg ICAgaWYgKGRyYXdCdXR0b25DZWxsKSB7CisgICAgICAgIGlmIChbY2VsbCBpc0tpbmRPZkNsYXNz OltOU1NsaWRlckNlbGwgY2xhc3NdXSkgeworICAgICAgICAgICAgLy8gRm9yIHNsaWRlciBjZWxs cywgZHJhdyBvbmx5IHRoZSBrbm9iLgorICAgICAgICAgICAgWyhOU1NsaWRlckNlbGwgKiljZWxs IGRyYXdLbm9iOnJlY3RdOworICAgICAgICB9IGVsc2UKKyAgICAgICAgICAgIFtjZWxsIGRyYXdX aXRoRnJhbWU6cmVjdCBpblZpZXc6dmlld107CisgICAgfQorICAgIGlmIChkcmF3Rm9jdXNSaW5n KQorICAgICAgICByZXR1cm4gZHJhd0NlbGxGb2N1c1JpbmcoY2VsbCwgcmVjdCwgdmlldyk7CisK KyAgICByZXR1cm4gZmFsc2U7Cit9CisKK2Jvb2wgVGhlbWVNYWM6OmRyYXdDZWxsT3JGb2N1c1Jp bmdXaXRoVmlld0ludG9Db250ZXh0KE5TQ2VsbCAqY2VsbCwgR3JhcGhpY3NDb250ZXh0JiBjb250 ZXh0LCBjb25zdCBGbG9hdFJlY3QmIHJlY3QsIE5TVmlldyAqdmlldywgYm9vbCBkcmF3QnV0dG9u Q2VsbCwgYm9vbCBkcmF3Rm9jdXNSaW5nLCBib29sIHVzZUltYWdlQnVmZmVyLCBmbG9hdCBkZXZp Y2VTY2FsZUZhY3RvcikKIHsKICAgICBBU1NFUlQoZHJhd0J1dHRvbkNlbGwgfHwgZHJhd0ZvY3Vz UmluZyk7CiAgICAgYm9vbCBuZWVkc1JlcGFpbnQgPSBmYWxzZTsKICAgICBpZiAodXNlSW1hZ2VC dWZmZXIpIHsKLSAgICAgICAgTlNSZWN0IGltYWdlQnVmZmVyRHJhd1JlY3QgPSBOU1JlY3QoRmxv YXRSZWN0KGJ1dHRvbkZvY3VzUmVjdE91dGxpbmVXaWR0aCwgYnV0dG9uRm9jdXNSZWN0T3V0bGlu ZVdpZHRoLCBpbmZsYXRlZFJlY3Qud2lkdGgoKSwgaW5mbGF0ZWRSZWN0LmhlaWdodCgpKSk7Ci0g ICAgICAgIHN0ZDo6dW5pcXVlX3B0cjxJbWFnZUJ1ZmZlcj4gaW1hZ2VCdWZmZXIgPSBJbWFnZUJ1 ZmZlcjo6Y3JlYXRlQ29tcGF0aWJsZUJ1ZmZlcihpbmZsYXRlZFJlY3Quc2l6ZSgpICsgMiAqIEZs b2F0U2l6ZShidXR0b25Gb2N1c1JlY3RPdXRsaW5lV2lkdGgsIGJ1dHRvbkZvY3VzUmVjdE91dGxp bmVXaWR0aCksIGRldmljZVNjYWxlRmFjdG9yLCBDb2xvclNwYWNlU1JHQiwgY29udGV4dCwgZmFs c2UpOworICAgICAgICBOU1JlY3QgaW1hZ2VCdWZmZXJEcmF3UmVjdCA9IE5TUmVjdChGbG9hdFJl Y3QoYnV0dG9uRm9jdXNSZWN0T3V0bGluZVdpZHRoLCBidXR0b25Gb2N1c1JlY3RPdXRsaW5lV2lk dGgsIHJlY3Qud2lkdGgoKSwgcmVjdC5oZWlnaHQoKSkpOworICAgICAgICBzdGQ6OnVuaXF1ZV9w dHI8SW1hZ2VCdWZmZXI+IGltYWdlQnVmZmVyID0gSW1hZ2VCdWZmZXI6OmNyZWF0ZUNvbXBhdGli bGVCdWZmZXIocmVjdC5zaXplKCkgKyAyICogRmxvYXRTaXplKGJ1dHRvbkZvY3VzUmVjdE91dGxp bmVXaWR0aCwgYnV0dG9uRm9jdXNSZWN0T3V0bGluZVdpZHRoKSwgZGV2aWNlU2NhbGVGYWN0b3Is IENvbG9yU3BhY2VTUkdCLCBjb250ZXh0LCBmYWxzZSk7CiAgICAgICAgIHsKICAgICAgICAgICAg IExvY2FsQ3VycmVudEdyYXBoaWNzQ29udGV4dCBsb2NhbENvbnRleHQoaW1hZ2VCdWZmZXItPmNv bnRleHQoKSk7Ci0gICAgICAgICAgICBpZiAoZHJhd0J1dHRvbkNlbGwpCi0gICAgICAgICAgICAg ICAgW2NlbGwgZHJhd1dpdGhGcmFtZTppbWFnZUJ1ZmZlckRyYXdSZWN0IGluVmlldzp2aWV3XTsK LSAgICAgICAgICAgIAotICAgICAgICAgICAgaWYgKGRyYXdGb2N1c1JpbmcpCi0gICAgICAgICAg ICAgICAgbmVlZHNSZXBhaW50ID0gZHJhd0NlbGxGb2N1c1JpbmcoY2VsbCwgaW1hZ2VCdWZmZXJE cmF3UmVjdCwgdmlldyk7CisgICAgICAgICAgICBuZWVkc1JlcGFpbnQgPSBkcmF3Q2VsbE9yRm9j dXNSaW5nSW50b1JlY3RXaXRoVmlldyhjZWxsLCBpbWFnZUJ1ZmZlckRyYXdSZWN0LCB2aWV3LCBk cmF3QnV0dG9uQ2VsbCwgZHJhd0ZvY3VzUmluZyk7CiAgICAgICAgIH0KLSAgICAgICAgY29udGV4 dC5kcmF3SW1hZ2VCdWZmZXIoaW1hZ2VCdWZmZXIuZ2V0KCksIENvbG9yU3BhY2VTUkdCLCBpbmZs YXRlZFJlY3QubG9jYXRpb24oKSAtIEZsb2F0U2l6ZShidXR0b25Gb2N1c1JlY3RPdXRsaW5lV2lk dGgsIGJ1dHRvbkZvY3VzUmVjdE91dGxpbmVXaWR0aCkpOworICAgICAgICBjb250ZXh0LmRyYXdJ bWFnZUJ1ZmZlcihpbWFnZUJ1ZmZlci5nZXQoKSwgQ29sb3JTcGFjZVNSR0IsIHJlY3QubG9jYXRp b24oKSAtIEZsb2F0U2l6ZShidXR0b25Gb2N1c1JlY3RPdXRsaW5lV2lkdGgsIGJ1dHRvbkZvY3Vz UmVjdE91dGxpbmVXaWR0aCkpOwogICAgICAgICByZXR1cm4gbmVlZHNSZXBhaW50OwogICAgIH0K ICAgICBpZiAoZHJhd0J1dHRvbkNlbGwpCi0gICAgICAgIFtjZWxsIGRyYXdXaXRoRnJhbWU6TlNS ZWN0KGluZmxhdGVkUmVjdCkgaW5WaWV3OnZpZXddOwotICAgIAotICAgIGlmIChkcmF3Rm9jdXNS aW5nKQotICAgICAgICBuZWVkc1JlcGFpbnQgPSBkcmF3Q2VsbEZvY3VzUmluZyhjZWxsLCBOU1Jl Y3QoaW5mbGF0ZWRSZWN0KSwgdmlldyk7CisgICAgICAgIG5lZWRzUmVwYWludCA9IGRyYXdDZWxs T3JGb2N1c1JpbmdJbnRvUmVjdFdpdGhWaWV3KGNlbGwsIE5TUmVjdChyZWN0KSwgdmlldywgZHJh d0J1dHRvbkNlbGwsIGRyYXdGb2N1c1JpbmcpOwogICAgIAogICAgIHJldHVybiBuZWVkc1JlcGFp bnQ7CiB9CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVN YWMubW0gYi9Tb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyVGhlbWVNYWMubW0KaW5kZXgg ZWRiMzNlNjQ3ZmJmYTI5ZTlhOWZiYWVhMzIzYzhiYmNlZDA4Yzc1NS4uNDk2NzY0NTk5MDAwNDU2 NzE5ZjZiZTJkZDJjODg3OTlhZTM0OGNhOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcmVu ZGVyaW5nL1JlbmRlclRoZW1lTWFjLm1tCisrKyBiL1NvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9S ZW5kZXJUaGVtZU1hYy5tbQpAQCAtMTU4MSw3ICsxNTgxLDExIEBAIGJvb2wgUmVuZGVyVGhlbWVN YWM6OnBhaW50U2xpZGVyVGh1bWIoY29uc3QgUmVuZGVyT2JqZWN0JiBvLCBjb25zdCBQYWludElu Zm8mIHBhCiAgICAgICAgIHBhaW50SW5mby5jb250ZXh0KCkudHJhbnNsYXRlKC11bnpvb21lZFJl Y3QueCgpLCAtdW56b29tZWRSZWN0LnkoKSk7CiAgICAgfQogCi0gICAgW3NsaWRlclRodW1iQ2Vs bCBkcmF3S25vYjp1bnpvb21lZFJlY3RdOworICAgIGJvb2wgc2hvdWxkRHJhd0NlbGwgPSB0cnVl OworICAgIGJvb2wgc2hvdWxkRHJhd0ZvY3VzUmluZyA9IGZhbHNlOworICAgIGZsb2F0IGRldmlj ZVNjYWxlRmFjdG9yID0gby5kb2N1bWVudCgpLnBhZ2UoKS0+ZGV2aWNlU2NhbGVGYWN0b3IoKTsK KyAgICBib29sIHNob3VsZFVzZUltYWdlQnVmZmVyID0gZGV2aWNlU2NhbGVGYWN0b3IgIT0gMSB8 fCB6b29tTGV2ZWwgIT0gMTsKKyAgICBUaGVtZU1hYzo6ZHJhd0NlbGxPckZvY3VzUmluZ1dpdGhW aWV3SW50b0NvbnRleHQoc2xpZGVyVGh1bWJDZWxsLCBwYWludEluZm8uY29udGV4dCgpLCB1bnpv b21lZFJlY3QsIHZpZXcsIHNob3VsZERyYXdDZWxsLCBzaG91bGREcmF3Rm9jdXNSaW5nLCBzaG91 bGRVc2VJbWFnZUJ1ZmZlciwgZGV2aWNlU2NhbGVGYWN0b3IpOwogICAgIFtzbGlkZXJUaHVtYkNl bGwgc2V0Q29udHJvbFZpZXc6bmlsXTsKIAogICAgIHJldHVybiBmYWxzZTsK