130985 2014-03-31 13:40:43 -0700 ThemeMac should use std::array instead of IntSize* for control sizes 2014-03-31 14:01:56 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 bdakin webkit-unassigned oldest_to_newest 995973 0 bdakin 2014-03-31 13:40:43 -0700 This bug is meant to address https://bugs.webkit.org/show_bug.cgi?id=130915#c2 ThemeMac should use std::array instead of IntSize* for control sizes. 995977 1 228199 bdakin 2014-03-31 13:55:04 -0700 Created attachment 228199 Patch 995983 2 228199 darin 2014-03-31 13:57:46 -0700 Comment on attachment 228199 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228199&action=review > Source/WebCore/platform/mac/ThemeMac.mm:158 > +static LengthSize sizeFromNSControlSize(NSControlSize nsControlSize, const LengthSize& zoomedSize, float zoomFactor,const std::array<IntSize, 3>& sizes) Missing space after comma here. > Source/WebCore/platform/mac/ThemeMac.mm:271 > + static const std::array<IntSize, 3> sizes = { {IntSize(14, 14), IntSize(12, 12), IntSize(10, 10)} }; I think there should be spaces inside the inner "{}" here too, not just the outer ones. 995984 3 228199 andersca 2014-03-31 13:57:57 -0700 Comment on attachment 228199 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228199&action=review > Source/WebCore/platform/mac/ThemeMac.mm:271 > + static const std::array<IntSize, 3> sizes = { {IntSize(14, 14), IntSize(12, 12), IntSize(10, 10)} }; Spaces after { and before }. 995986 4 bdakin 2014-03-31 14:01:56 -0700 (In reply to comment #2) > (From update of attachment 228199 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=228199&action=review > > > Source/WebCore/platform/mac/ThemeMac.mm:158 > > +static LengthSize sizeFromNSControlSize(NSControlSize nsControlSize, const LengthSize& zoomedSize, float zoomFactor,const std::array<IntSize, 3>& sizes) > > Missing space after comma here. > Ooh, good catch. > > Source/WebCore/platform/mac/ThemeMac.mm:271 > > + static const std::array<IntSize, 3> sizes = { {IntSize(14, 14), IntSize(12, 12), IntSize(10, 10)} }; > > I think there should be spaces inside the inner "{}" here too, not just the outer ones. Okay! Fixed. Thanks, Darin! http://trac.webkit.org/changeset/166523 228199 2014-03-31 13:55:04 -0700 2014-03-31 13:57:57 -0700 Patch for-review.txt text/plain 5366 bdakin SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2NjUyMSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDE0LTAzLTMxICBCZXRoIERh a2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBUaGVtZU1hYyBzaG91bGQgdXNlIHN0 ZDo6YXJyYXkgaW5zdGVhZCBvZiBJbnRTaXplKiBmb3IgY29udHJvbCBzaXplcworICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTMwOTg1CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgUmVwbGFjZSB1c2VzIG9mIGNv bnN0IEludFNpemUqIHdpdGggY29uc3Qgc3RkOjphcnJheTxJbnRTaXplLCAzPgorICAgICAgICAq IHBsYXRmb3JtL21hYy9UaGVtZU1hYy5tbToKKyAgICAgICAgKFdlYkNvcmU6OnNpemVGcm9tTlND b250cm9sU2l6ZSk6CisgICAgICAgIChXZWJDb3JlOjpzaXplRnJvbUZvbnQpOgorICAgICAgICAo V2ViQ29yZTo6Y29udHJvbFNpemVGcm9tUGl4ZWxTaXplKToKKyAgICAgICAgKFdlYkNvcmU6OnNl dENvbnRyb2xTaXplKToKKyAgICAgICAgKFdlYkNvcmU6OmNoZWNrYm94U2l6ZXMpOgorICAgICAg ICAoV2ViQ29yZTo6cmFkaW9TaXplcyk6CisgICAgICAgIChXZWJDb3JlOjpidXR0b25TaXplcyk6 CisgICAgICAgIChXZWJDb3JlOjpzZXRVcEJ1dHRvbkNlbGwpOgorICAgICAgICAoV2ViQ29yZTo6 c3RlcHBlclNpemVzKToKKwogMjAxNC0wMy0zMSAgQW5kcmVhcyBLbGluZyAgPGFrbGluZ0BhcHBs ZS5jb20+CiAKICAgICAgICAgQWx3YXlzIGlubGluZSB0b0pTKCkgZm9yIE5vZGVMaXN0LgpJbmRl eDogU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWFjL1RoZW1lTWFjLm1tCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL21hYy9UaGVtZU1hYy5tbQkocmV2aXNpb24gMTY2NTE0 KQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vbWFjL1RoZW1lTWFjLm1tCSh3b3JraW5nIGNv cHkpCkBAIC0xNTUsNyArMTU1LDcgQEAgc3RhdGljIE5TQ29udHJvbFNpemUgY29udHJvbFNpemVG b3JGb250KAogICAgIHJldHVybiBOU01pbmlDb250cm9sU2l6ZTsKIH0KIAotc3RhdGljIExlbmd0 aFNpemUgc2l6ZUZyb21OU0NvbnRyb2xTaXplKE5TQ29udHJvbFNpemUgbnNDb250cm9sU2l6ZSwg Y29uc3QgTGVuZ3RoU2l6ZSYgem9vbWVkU2l6ZSwgZmxvYXQgem9vbUZhY3RvciwgY29uc3QgSW50 U2l6ZSogc2l6ZXMpCitzdGF0aWMgTGVuZ3RoU2l6ZSBzaXplRnJvbU5TQ29udHJvbFNpemUoTlND b250cm9sU2l6ZSBuc0NvbnRyb2xTaXplLCBjb25zdCBMZW5ndGhTaXplJiB6b29tZWRTaXplLCBm bG9hdCB6b29tRmFjdG9yLGNvbnN0IHN0ZDo6YXJyYXk8SW50U2l6ZSwgMz4mIHNpemVzKQogewog ICAgIEludFNpemUgY29udHJvbFNpemUgPSBzaXplc1tuc0NvbnRyb2xTaXplXTsKICAgICBpZiAo em9vbUZhY3RvciAhPSAxLjBmKQpAQCAtMTY4LDEyICsxNjgsMTIgQEAgc3RhdGljIExlbmd0aFNp emUgc2l6ZUZyb21OU0NvbnRyb2xTaXplKAogICAgIHJldHVybiByZXN1bHQ7CiB9CiAKLXN0YXRp YyBMZW5ndGhTaXplIHNpemVGcm9tRm9udChjb25zdCBGb250JiBmb250LCBjb25zdCBMZW5ndGhT aXplJiB6b29tZWRTaXplLCBmbG9hdCB6b29tRmFjdG9yLCBjb25zdCBJbnRTaXplKiBzaXplcykK K3N0YXRpYyBMZW5ndGhTaXplIHNpemVGcm9tRm9udChjb25zdCBGb250JiBmb250LCBjb25zdCBM ZW5ndGhTaXplJiB6b29tZWRTaXplLCBmbG9hdCB6b29tRmFjdG9yLCBjb25zdCBzdGQ6OmFycmF5 PEludFNpemUsIDM+JiBzaXplcykKIHsKICAgICByZXR1cm4gc2l6ZUZyb21OU0NvbnRyb2xTaXpl KGNvbnRyb2xTaXplRm9yRm9udChmb250KSwgem9vbWVkU2l6ZSwgem9vbUZhY3Rvciwgc2l6ZXMp OwogfQogCi1zdGF0aWMgQ29udHJvbFNpemUgY29udHJvbFNpemVGcm9tUGl4ZWxTaXplKGNvbnN0 IEludFNpemUqIHNpemVzLCBjb25zdCBJbnRTaXplJiBtaW5ab29tZWRTaXplLCBmbG9hdCB6b29t RmFjdG9yKQorc3RhdGljIENvbnRyb2xTaXplIGNvbnRyb2xTaXplRnJvbVBpeGVsU2l6ZShjb25z dCBzdGQ6OmFycmF5PEludFNpemUsIDM+JiBzaXplcywgY29uc3QgSW50U2l6ZSYgbWluWm9vbWVk U2l6ZSwgZmxvYXQgem9vbUZhY3RvcikKIHsKICAgICBpZiAobWluWm9vbWVkU2l6ZS53aWR0aCgp ID49IHN0YXRpY19jYXN0PGludD4oc2l6ZXNbTlNSZWd1bGFyQ29udHJvbFNpemVdLndpZHRoKCkg KiB6b29tRmFjdG9yKSAmJgogICAgICAgICBtaW5ab29tZWRTaXplLmhlaWdodCgpID49IHN0YXRp Y19jYXN0PGludD4oc2l6ZXNbTlNSZWd1bGFyQ29udHJvbFNpemVdLmhlaWdodCgpICogem9vbUZh Y3RvcikpCkBAIC0xODQsNyArMTg0LDcgQEAgc3RhdGljIENvbnRyb2xTaXplIGNvbnRyb2xTaXpl RnJvbVBpeGVsUwogICAgIHJldHVybiBOU01pbmlDb250cm9sU2l6ZTsKIH0KIAotc3RhdGljIHZv aWQgc2V0Q29udHJvbFNpemUoTlNDZWxsKiBjZWxsLCBjb25zdCBJbnRTaXplKiBzaXplcywgY29u c3QgSW50U2l6ZSYgbWluWm9vbWVkU2l6ZSwgZmxvYXQgem9vbUZhY3RvcikKK3N0YXRpYyB2b2lk IHNldENvbnRyb2xTaXplKE5TQ2VsbCogY2VsbCwgY29uc3Qgc3RkOjphcnJheTxJbnRTaXplLCAz PiYgc2l6ZXMsIGNvbnN0IEludFNpemUmIG1pblpvb21lZFNpemUsIGZsb2F0IHpvb21GYWN0b3Ip CiB7CiAgICAgQ29udHJvbFNpemUgc2l6ZSA9IGNvbnRyb2xTaXplRnJvbVBpeGVsU2l6ZShzaXpl cywgbWluWm9vbWVkU2l6ZSwgem9vbUZhY3Rvcik7CiAgICAgaWYgKHNpemUgIT0gW2NlbGwgY29u dHJvbFNpemVdKSAvLyBPbmx5IHVwZGF0ZSBpZiB3ZSBoYXZlIHRvLCBzaW5jZSBBcHBLaXQgZG9l cyB3b3JrIGV2ZW4gaWYgdGhlIHNpemUgaXMgdGhlIHNhbWUuCkBAIC0yNjYsOSArMjY2LDkgQEAg c3RhdGljIEludFJlY3QgaW5mbGF0ZVJlY3QoY29uc3QgSW50UmVjdAogCiAvLyBDaGVja2JveGVz IGFuZCByYWRpbyBidXR0b25zCiAKLXN0YXRpYyBjb25zdCBJbnRTaXplKiBjaGVja2JveFNpemVz KCkKK3N0YXRpYyBjb25zdCBzdGQ6OmFycmF5PEludFNpemUsIDM+JiBjaGVja2JveFNpemVzKCkK IHsKLSAgICBzdGF0aWMgY29uc3QgSW50U2l6ZSBzaXplc1szXSA9IHsgSW50U2l6ZSgxNCwgMTQp LCBJbnRTaXplKDEyLCAxMiksIEludFNpemUoMTAsIDEwKSB9OworICAgIHN0YXRpYyBjb25zdCBz dGQ6OmFycmF5PEludFNpemUsIDM+IHNpemVzID0geyB7SW50U2l6ZSgxNCwgMTQpLCBJbnRTaXpl KDEyLCAxMiksIEludFNpemUoMTAsIDEwKX0gfTsKICAgICByZXR1cm4gc2l6ZXM7CiB9CiAKQEAg LTI5NSw5ICsyOTUsOSBAQCBzdGF0aWMgTGVuZ3RoU2l6ZSBjaGVja2JveFNpemUoY29uc3QgRm9u CiAgICAgCiAvLyBSYWRpbyBCdXR0b25zCiAKLXN0YXRpYyBjb25zdCBJbnRTaXplKiByYWRpb1Np emVzKCkKK3N0YXRpYyBjb25zdCBzdGQ6OmFycmF5PEludFNpemUsIDM+JiByYWRpb1NpemVzKCkK IHsKLSAgICBzdGF0aWMgY29uc3QgSW50U2l6ZSBzaXplc1szXSA9IHsgSW50U2l6ZSgxNCwgMTUp LCBJbnRTaXplKDEyLCAxMyksIEludFNpemUoMTAsIDEwKSB9OworICAgIHN0YXRpYyBjb25zdCBz dGQ6OmFycmF5PEludFNpemUsIDM+IHNpemVzID0geyB7SW50U2l6ZSgxNCwgMTUpLCBJbnRTaXpl KDEyLCAxMyksIEludFNpemUoMTAsIDEwKX0gfTsKICAgICByZXR1cm4gc2l6ZXM7CiB9CiAKQEAg LTQ0Miw5ICs0NDIsOSBAQCBzdGF0aWMgdm9pZCBwYWludFRvZ2dsZUJ1dHRvbihDb250cm9sUGFy CiAvLyBCdXR0b25zCiAKIC8vIEJ1dHRvbnMgcmVhbGx5IG9ubHkgY29uc3RyYWluIGhlaWdodC4g VGhleSByZXNwZWN0IHdpZHRoLgotc3RhdGljIGNvbnN0IEludFNpemUqIGJ1dHRvblNpemVzKCkK K3N0YXRpYyBjb25zdCBzdGQ6OmFycmF5PEludFNpemUsIDM+JiBidXR0b25TaXplcygpCiB7Ci0g ICAgc3RhdGljIGNvbnN0IEludFNpemUgc2l6ZXNbM10gPSB7IEludFNpemUoMCwgMjEpLCBJbnRT aXplKDAsIDE4KSwgSW50U2l6ZSgwLCAxNSkgfTsKKyAgICBzdGF0aWMgY29uc3Qgc3RkOjphcnJh eTxJbnRTaXplLCAzPiBzaXplcyA9IHsge0ludFNpemUoMCwgMjEpLCBJbnRTaXplKDAsIDE4KSwg SW50U2l6ZSgwLCAxNSl9IH07CiAgICAgcmV0dXJuIHNpemVzOwogfQogCkBAIC00NzQsNyArNDc0 LDcgQEAgc3RhdGljIE5TQnV0dG9uQ2VsbCAqbGVha0J1dHRvbkNlbGwoQnV0dAogc3RhdGljIHZv aWQgc2V0VXBCdXR0b25DZWxsKE5TQnV0dG9uQ2VsbCAqY2VsbCwgQ29udHJvbFBhcnQgcGFydCwg Y29uc3QgQ29udHJvbFN0YXRlcyogc3RhdGVzLCBjb25zdCBJbnRSZWN0JiB6b29tZWRSZWN0LCBm bG9hdCB6b29tRmFjdG9yKQogewogICAgIC8vIFNldCB0aGUgY29udHJvbCBzaXplIGJhc2VkIG9m ZiB0aGUgcmVjdGFuZ2xlIHdlJ3JlIHBhaW50aW5nIGludG8uCi0gICAgY29uc3QgSW50U2l6ZSog c2l6ZXMgPSBidXR0b25TaXplcygpOworICAgIGNvbnN0IHN0ZDo6YXJyYXk8SW50U2l6ZSwgMz4m IHNpemVzID0gYnV0dG9uU2l6ZXMoKTsKICAgICBpZiAocGFydCA9PSBTcXVhcmVCdXR0b25QYXJ0 IHx8IHpvb21lZFJlY3QuaGVpZ2h0KCkgPiBidXR0b25TaXplcygpW05TUmVndWxhckNvbnRyb2xT aXplXS5oZWlnaHQoKSAqIHpvb21GYWN0b3IpIHsKICAgICAgICAgLy8gVXNlIHRoZSBzcXVhcmUg YnV0dG9uCiAgICAgICAgIGlmIChbY2VsbCBiZXplbFN0eWxlXSAhPSBOU1NoYWRvd2xlc3NTcXVh cmVCZXplbFN0eWxlKQpAQCAtNTYwLDkgKzU2MCw5IEBAIHN0YXRpYyB2b2lkIHBhaW50QnV0dG9u KENvbnRyb2xQYXJ0IHBhcnQKIAogLy8gU3RlcHBlcgogCi1zdGF0aWMgY29uc3QgSW50U2l6ZSog c3RlcHBlclNpemVzKCkKK3N0YXRpYyBjb25zdCBzdGQ6OmFycmF5PEludFNpemUsIDM+JiBzdGVw cGVyU2l6ZXMoKQogewotICAgIHN0YXRpYyBjb25zdCBJbnRTaXplIHNpemVzWzNdID0geyBJbnRT aXplKDE5LCAyNyksIEludFNpemUoMTUsIDIyKSwgSW50U2l6ZSgxMywgMTUpIH07CisgICAgc3Rh dGljIGNvbnN0IHN0ZDo6YXJyYXk8SW50U2l6ZSwgMz4gc2l6ZXMgPSB7IHtJbnRTaXplKDE5LCAy NyksIEludFNpemUoMTUsIDIyKSwgSW50U2l6ZSgxMywgMTUpfSB9OwogICAgIHJldHVybiBzaXpl czsKIH0KIAo=