30319 2009-10-12 17:57:01 -0700 Lines on Chrome's horizontal scrollbars should be vertical 2009-10-13 20:52:16 -0700 1 1 1 Unclassified WebKit Forms 528+ (Nightly build) PC Linux RESOLVED FIXED P2 Normal --- 1 evan evan commit-queue levin tony oldest_to_newest 154220 0 evan 2009-10-12 17:57:01 -0700 On Linux, the "grippies" on Webkit's horizontal scrollbars are oriented incorrectly. They should be vertical lines, perpendicular to the scrollbar's direction of travel; instead, the same code is used to draw horizontal lines for both horizontal and vertical scrollbars. 154224 1 tony 2009-10-12 18:05:32 -0700 If you're feeling ambitious, you could change the painting of sliders too since it's a copy/paste of this code. 154231 2 41080 evan 2009-10-12 18:22:38 -0700 Created attachment 41080 fix lines 154501 3 41080 pkasting 2009-10-13 17:29:22 -0700 Comment on attachment 41080 fix lines > + if (vertical) { > + drawHorizLine(canvas, midx - 2, midx + 2, midy, paint); > + drawHorizLine(canvas, midx - 2, midx + 2, midy - 3, paint); > + drawHorizLine(canvas, midx - 2, midx + 2, midy + 3, paint); Nit: Why not do these in (midy - 3, midy, midy + 3) order like the vertical lines? Tony's comment about also changing the slider code seems apt here. 154502 4 evan 2009-10-13 17:31:19 -0700 I will fix the ordering. To ease rebaselining, I'd prefer to do the slider change in a separate step. 154505 5 41142 evan 2009-10-13 17:32:34 -0700 Created attachment 41142 fix drawing of lines 154506 6 41142 levin 2009-10-13 17:52:28 -0700 Comment on attachment 41142 fix drawing of lines r- on this patch for 3 reasons: 1. It does more than it says. It changes how the horizontal lines are drawn without explanation (nothing in the changelog and the bug only mentions the other direction). 2. It uses a lot of "magic" numbersy. It would be nice to make constants out of some of them to help inform the reader what they are. 3. It is unclear if there are any layout tests which cover this change. 154516 7 41145 evan 2009-10-13 19:12:09 -0700 Created attachment 41145 fix drawing of lines 154517 8 evan 2009-10-13 19:13:16 -0700 Thanks for the review! (In reply to comment #6) > 1. It does more than it says. It changes how the horizontal lines are drawn > without explanation (nothing in the changelog and the bug only mentions the > other direction). Updated the changelog. > 2. It uses a lot of "magic" numbersy. It would be nice to make constants out of > some of them to help inform the reader what they are. I added some constants, but I was only adjusting the way the code was already and think they make the code less readable. > 3. It is unclear if there are any layout tests which cover this change. I noted in the changelog that this is covered by existing tests. 154526 9 41145 commit-queue 2009-10-13 20:52:12 -0700 Comment on attachment 41145 fix drawing of lines Clearing flags on attachment: 41145 Committed r49553: <http://trac.webkit.org/changeset/49553> 154527 10 commit-queue 2009-10-13 20:52:16 -0700 All reviewed patches have been landed. Closing bug. 41080 2009-10-12 18:22:38 -0700 2009-10-13 17:32:32 -0700 fix lines bug-30319-20091012182237.patch text/plain 1770 evan ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MmNlNjM1MC4uNGU3MTM3ZiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAwOS0xMC0xMiAgRXZhbiBNYXJ0aW4g IDxldmFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBNYWtlIGdyaXBweSBsaW5lcyB2ZXJ0aWNhbCBvbiBob3Jpem9udGFsIHNj cm9sbGJhcnMgaW4gTGludXggQ2hyb21lLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0zMDMxOQorCisgICAgICAgICogcGxhdGZvcm0vY2hyb21pdW0v U2Nyb2xsYmFyVGhlbWVDaHJvbWl1bUxpbnV4LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNjcm9s bGJhclRoZW1lQ2hyb21pdW1MaW51eDo6cGFpbnRUaHVtYik6CisKIDIwMDktMTAtMDkgIEFkYW0g QmFydGggIDxhYmFydGhAd2Via2l0Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBB ZGxlci4KZGlmZiAtLWdpdCBhL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vU2Nyb2xsYmFyVGhl bWVDaHJvbWl1bUxpbnV4LmNwcCBiL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vU2Nyb2xsYmFy VGhlbWVDaHJvbWl1bUxpbnV4LmNwcAppbmRleCBlZDhiZjM2Li4xN2M5MmU4IDEwMDY0NAotLS0g YS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9taXVtL1Njcm9sbGJhclRoZW1lQ2hyb21pdW1MaW51eC5j cHAKKysrIGIvV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9TY3JvbGxiYXJUaGVtZUNocm9taXVt TGludXguY3BwCkBAIC0xNDAsOSArMTQwLDE1IEBAIHZvaWQgU2Nyb2xsYmFyVGhlbWVDaHJvbWl1 bUxpbnV4OjpwYWludFRodW1iKEdyYXBoaWNzQ29udGV4dCogZ2MsIFNjcm9sbGJhciogc2NyCiAK ICAgICBpZiAocmVjdC5oZWlnaHQoKSA+IDEwICYmIHJlY3Qud2lkdGgoKSA+IDEwKSB7CiAgICAg ICAgIHBhaW50LnNldEFSR0IoMHhmZiwgMHg5ZCwgMHg5NiwgMHg4ZSk7Ci0gICAgICAgIGRyYXdI b3JpekxpbmUoY2FudmFzLCBtaWR4IC0gMSwgbWlkeCArIDMsIG1pZHksIHBhaW50KTsKLSAgICAg ICAgZHJhd0hvcml6TGluZShjYW52YXMsIG1pZHggLSAxLCBtaWR4ICsgMywgbWlkeSAtIDMsIHBh aW50KTsKLSAgICAgICAgZHJhd0hvcml6TGluZShjYW52YXMsIG1pZHggLSAxLCBtaWR4ICsgMywg bWlkeSArIDMsIHBhaW50KTsKKyAgICAgICAgaWYgKHZlcnRpY2FsKSB7CisgICAgICAgICAgICBk cmF3SG9yaXpMaW5lKGNhbnZhcywgbWlkeCAtIDIsIG1pZHggKyAyLCBtaWR5LCAgICAgcGFpbnQp OworICAgICAgICAgICAgZHJhd0hvcml6TGluZShjYW52YXMsIG1pZHggLSAyLCBtaWR4ICsgMiwg bWlkeSAtIDMsIHBhaW50KTsKKyAgICAgICAgICAgIGRyYXdIb3JpekxpbmUoY2FudmFzLCBtaWR4 IC0gMiwgbWlkeCArIDIsIG1pZHkgKyAzLCBwYWludCk7CisgICAgICAgIH0gZWxzZSB7CisgICAg ICAgICAgICBkcmF3VmVydExpbmUoY2FudmFzLCBtaWR4IC0gMywgbWlkeSAtIDIsIG1pZHkgKyAy LCBwYWludCk7CisgICAgICAgICAgICBkcmF3VmVydExpbmUoY2FudmFzLCBtaWR4LCAgICAgbWlk eSAtIDIsIG1pZHkgKyAyLCBwYWludCk7CisgICAgICAgICAgICBkcmF3VmVydExpbmUoY2FudmFz LCBtaWR4ICsgMywgbWlkeSAtIDIsIG1pZHkgKyAyLCBwYWludCk7CisgICAgICAgIH0KICAgICB9 CiB9 41142 2009-10-13 17:32:34 -0700 2009-10-13 19:12:06 -0700 fix drawing of lines bug-30319-20091013173233.patch text/plain 1786 evan ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg M2VkNGYxNy4uOGYwOTdkOCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAwOS0xMC0xMiAgRXZhbiBNYXJ0aW4g IDxldmFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBNYWtlIGdyaXBweSBsaW5lcyB2ZXJ0aWNhbCBvbiBob3Jpem9udGFsIHNj cm9sbGJhcnMgaW4gTGludXggQ2hyb21lLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0zMDMxOQorCisgICAgICAgICogcGxhdGZvcm0vY2hyb21pdW0v U2Nyb2xsYmFyVGhlbWVDaHJvbWl1bUxpbnV4LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNjcm9s bGJhclRoZW1lQ2hyb21pdW1MaW51eDo6cGFpbnRUaHVtYik6CisKIDIwMDktMTAtMTMgIEdpcmlz aCBSYW1ha3Jpc2huYW4gIDxnaXJpc2hAZm9yd2FyZGJpYXMuaW4+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgU2ltb24gSGF1c21hbm4uCmRpZmYgLS1naXQgYS9XZWJDb3JlL3BsYXRmb3JtL2Nocm9t aXVtL1Njcm9sbGJhclRoZW1lQ2hyb21pdW1MaW51eC5jcHAgYi9XZWJDb3JlL3BsYXRmb3JtL2No cm9taXVtL1Njcm9sbGJhclRoZW1lQ2hyb21pdW1MaW51eC5jcHAKaW5kZXggZWQ4YmYzNi4uZTli ODQ2MiAxMDA2NDQKLS0tIGEvV2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9TY3JvbGxiYXJUaGVt ZUNocm9taXVtTGludXguY3BwCisrKyBiL1dlYkNvcmUvcGxhdGZvcm0vY2hyb21pdW0vU2Nyb2xs YmFyVGhlbWVDaHJvbWl1bUxpbnV4LmNwcApAQCAtMTQwLDkgKzE0MCwxNSBAQCB2b2lkIFNjcm9s bGJhclRoZW1lQ2hyb21pdW1MaW51eDo6cGFpbnRUaHVtYihHcmFwaGljc0NvbnRleHQqIGdjLCBT Y3JvbGxiYXIqIHNjcgogCiAgICAgaWYgKHJlY3QuaGVpZ2h0KCkgPiAxMCAmJiByZWN0LndpZHRo KCkgPiAxMCkgewogICAgICAgICBwYWludC5zZXRBUkdCKDB4ZmYsIDB4OWQsIDB4OTYsIDB4OGUp OwotICAgICAgICBkcmF3SG9yaXpMaW5lKGNhbnZhcywgbWlkeCAtIDEsIG1pZHggKyAzLCBtaWR5 LCBwYWludCk7Ci0gICAgICAgIGRyYXdIb3JpekxpbmUoY2FudmFzLCBtaWR4IC0gMSwgbWlkeCAr IDMsIG1pZHkgLSAzLCBwYWludCk7Ci0gICAgICAgIGRyYXdIb3JpekxpbmUoY2FudmFzLCBtaWR4 IC0gMSwgbWlkeCArIDMsIG1pZHkgKyAzLCBwYWludCk7CisgICAgICAgIGlmICh2ZXJ0aWNhbCkg eworICAgICAgICAgICAgZHJhd0hvcml6TGluZShjYW52YXMsIG1pZHggLSAyLCBtaWR4ICsgMiwg bWlkeSAtIDMsIHBhaW50KTsKKyAgICAgICAgICAgIGRyYXdIb3JpekxpbmUoY2FudmFzLCBtaWR4 IC0gMiwgbWlkeCArIDIsIG1pZHksICAgICBwYWludCk7CisgICAgICAgICAgICBkcmF3SG9yaXpM aW5lKGNhbnZhcywgbWlkeCAtIDIsIG1pZHggKyAyLCBtaWR5ICsgMywgcGFpbnQpOworICAgICAg ICB9IGVsc2UgeworICAgICAgICAgICAgZHJhd1ZlcnRMaW5lKGNhbnZhcywgbWlkeCAtIDMsIG1p ZHkgLSAyLCBtaWR5ICsgMiwgcGFpbnQpOworICAgICAgICAgICAgZHJhd1ZlcnRMaW5lKGNhbnZh cywgbWlkeCwgICAgIG1pZHkgLSAyLCBtaWR5ICsgMiwgcGFpbnQpOworICAgICAgICAgICAgZHJh d1ZlcnRMaW5lKGNhbnZhcywgbWlkeCArIDMsIG1pZHkgLSAyLCBtaWR5ICsgMiwgcGFpbnQpOwor ICAgICAgICB9CiAgICAgfQogfQ== 41145 2009-10-13 19:12:09 -0700 2009-10-13 20:52:11 -0700 fix drawing of lines bug-30319-20091013191208.patch text/plain 2322 evan ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg M2VkNGYxNy4uM2Q4ZGZkNCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvV2Vi Q29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOCBAQAorMjAwOS0xMC0xMiAgRXZhbiBNYXJ0aW4g IDxldmFuQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBNYWtlIGdyaXBweSBsaW5lcyB2ZXJ0aWNhbCBvbiBob3Jpem9udGFsIHNj cm9sbGJhcnMgaW4gTGludXggQ2hyb21lLgorICAgICAgICBXaGlsZSB3ZSdyZSByZWJhc2VsaW5p bmcgc2Nyb2xsYmFycywgZml4IGFuIG9mZiBieSBvbmUgaW4gdGhlIHZlcnRpY2FsCisgICAgICAg IHNjcm9sbGJhciByZW5kZXJpbmcgdG9vLgorCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQu b3JnL3Nob3dfYnVnLmNnaT9pZD0zMDMxOQorCisgICAgICAgIFRlc3RzOiB0aGlzIGlzIGNvdmVy ZWQgYnkgZXZlcnkgcGl4ZWwgdGVzdCBpbnZvbHZpbmcgc2Nyb2xsYmFycy4KKworICAgICAgICAq IHBsYXRmb3JtL2Nocm9taXVtL1Njcm9sbGJhclRoZW1lQ2hyb21pdW1MaW51eC5jcHA6CisgICAg ICAgIChXZWJDb3JlOjpTY3JvbGxiYXJUaGVtZUNocm9taXVtTGludXg6OnBhaW50VGh1bWIpOgor CiAyMDA5LTEwLTEzICBHaXJpc2ggUmFtYWtyaXNobmFuICA8Z2lyaXNoQGZvcndhcmRiaWFzLmlu PgogCiAgICAgICAgIFJldmlld2VkIGJ5IFNpbW9uIEhhdXNtYW5uLgpkaWZmIC0tZ2l0IGEvV2Vi Q29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9TY3JvbGxiYXJUaGVtZUNocm9taXVtTGludXguY3BwIGIv V2ViQ29yZS9wbGF0Zm9ybS9jaHJvbWl1bS9TY3JvbGxiYXJUaGVtZUNocm9taXVtTGludXguY3Bw CmluZGV4IGVkOGJmMzYuLjY0ZjU4YzQgMTAwNjQ0Ci0tLSBhL1dlYkNvcmUvcGxhdGZvcm0vY2hy b21pdW0vU2Nyb2xsYmFyVGhlbWVDaHJvbWl1bUxpbnV4LmNwcAorKysgYi9XZWJDb3JlL3BsYXRm b3JtL2Nocm9taXVtL1Njcm9sbGJhclRoZW1lQ2hyb21pdW1MaW51eC5jcHAKQEAgLTE0MCw5ICsx NDAsMTcgQEAgdm9pZCBTY3JvbGxiYXJUaGVtZUNocm9taXVtTGludXg6OnBhaW50VGh1bWIoR3Jh cGhpY3NDb250ZXh0KiBnYywgU2Nyb2xsYmFyKiBzY3IKIAogICAgIGlmIChyZWN0LmhlaWdodCgp ID4gMTAgJiYgcmVjdC53aWR0aCgpID4gMTApIHsKICAgICAgICAgcGFpbnQuc2V0QVJHQigweGZm LCAweDlkLCAweDk2LCAweDhlKTsKLSAgICAgICAgZHJhd0hvcml6TGluZShjYW52YXMsIG1pZHgg LSAxLCBtaWR4ICsgMywgbWlkeSwgcGFpbnQpOwotICAgICAgICBkcmF3SG9yaXpMaW5lKGNhbnZh cywgbWlkeCAtIDEsIG1pZHggKyAzLCBtaWR5IC0gMywgcGFpbnQpOwotICAgICAgICBkcmF3SG9y aXpMaW5lKGNhbnZhcywgbWlkeCAtIDEsIG1pZHggKyAzLCBtaWR5ICsgMywgcGFpbnQpOworICAg ICAgICBjb25zdCBpbnQgZ3JpcHB5SGFsZldpZHRoID0gMjsKKyAgICAgICAgY29uc3QgaW50IGlu dGVyR3JpcHB5T2Zmc2V0ID0gMzsKKyAgICAgICAgaWYgKHZlcnRpY2FsKSB7CisgICAgICAgICAg ICBkcmF3SG9yaXpMaW5lKGNhbnZhcywgbWlkeCAtIGdyaXBweUhhbGZXaWR0aCwgbWlkeCArIGdy aXBweUhhbGZXaWR0aCwgbWlkeSAtIGludGVyR3JpcHB5T2Zmc2V0LCBwYWludCk7CisgICAgICAg ICAgICBkcmF3SG9yaXpMaW5lKGNhbnZhcywgbWlkeCAtIGdyaXBweUhhbGZXaWR0aCwgbWlkeCAr IGdyaXBweUhhbGZXaWR0aCwgbWlkeSwgICAgICAgICAgICAgICAgICAgICBwYWludCk7CisgICAg ICAgICAgICBkcmF3SG9yaXpMaW5lKGNhbnZhcywgbWlkeCAtIGdyaXBweUhhbGZXaWR0aCwgbWlk eCArIGdyaXBweUhhbGZXaWR0aCwgbWlkeSArIGludGVyR3JpcHB5T2Zmc2V0LCBwYWludCk7Cisg ICAgICAgIH0gZWxzZSB7CisgICAgICAgICAgICBkcmF3VmVydExpbmUoY2FudmFzLCBtaWR4IC0g aW50ZXJHcmlwcHlPZmZzZXQsIG1pZHkgLSBncmlwcHlIYWxmV2lkdGgsIG1pZHkgKyBncmlwcHlI YWxmV2lkdGgsIHBhaW50KTsKKyAgICAgICAgICAgIGRyYXdWZXJ0TGluZShjYW52YXMsIG1pZHgs ICAgICAgICAgICAgICAgICAgICAgbWlkeSAtIGdyaXBweUhhbGZXaWR0aCwgbWlkeSArIGdyaXBw eUhhbGZXaWR0aCwgcGFpbnQpOworICAgICAgICAgICAgZHJhd1ZlcnRMaW5lKGNhbnZhcywgbWlk eCArIGludGVyR3JpcHB5T2Zmc2V0LCBtaWR5IC0gZ3JpcHB5SGFsZldpZHRoLCBtaWR5ICsgZ3Jp cHB5SGFsZldpZHRoLCBwYWludCk7CisgICAgICAgIH0KICAgICB9CiB9