92737 2012-07-31 04:03:59 -0700 Enable kerning on Android 2012-08-09 11:41:25 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Android RESOLVED FIXED P2 Normal --- 66687 0 husky webkit-unassigned abarth husky peter webkit.review.bot oldest_to_newest 682900 0 husky 2012-07-31 04:03:59 -0700 Kerning was disabled in http://trac.webkit.org/changeset/121609, which includes this code: #if OS(ANDROID) // Kerning does not currently work on Android. m_item.shaperFlags = HB_ShaperFlag_NoKerning; #endif 690468 1 157245 husky 2012-08-08 10:25:19 -0700 Created attachment 157245 Patch 690470 2 husky 2012-08-08 10:29:04 -0700 +abarth Adam, you did the last change to this file, so if you have time could you take a look at this, please? I'm having trouble tracking down what the original problem actually was. We had an old bug where the character spacing was messed up in text styled with "text-rendering: optimizelegibility", and that one is definitely fixed now. I'll run all the downstream tests to check that this doesn't cause problems. 690471 3 157245 peter 2012-08-08 10:29:50 -0700 Comment on attachment 157245 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157245&action=review Looks good to me, although I'd like a clarification in the ChangeLog re: testing. > Source/WebCore/ChangeLog:12 > + Testing: existing UI tests (not all unforked yet) Kerning is something that also comes up in layout tests, will this influence our results there? > Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:-83 > -#if OS(ANDROID) Nit: since this change is specific to Chromium and doesn't touch other platforms, you could prefix the bug's subject with [Chromium]. 690493 4 157245 abarth 2012-08-08 10:40:28 -0700 Comment on attachment 157245 Patch Please address Peter's comments before landing. 691407 5 157245 husky 2012-08-09 05:20:52 -0700 Comment on attachment 157245 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157245&action=review >> Source/WebCore/ChangeLog:12 >> + Testing: existing UI tests (not all unforked yet) > > Kerning is something that also comes up in layout tests, will this influence our results there? Potentially yes, although most layout tests try to avoid dependencies on font metrics. Simplified to "tests". >> Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:-83 >> -#if OS(ANDROID) > > Nit: since this change is specific to Chromium and doesn't touch other platforms, you could prefix the bug's subject with [Chromium]. Done 691409 6 157443 husky 2012-08-09 05:21:32 -0700 Created attachment 157443 Patch 691436 7 157443 peter 2012-08-09 05:36:04 -0700 Comment on attachment 157443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157443&action=review > Source/WebCore/ChangeLog:6 > + Reviewed by NOBODY (OOPS!). Adam already r+'ed your patch :-). If you change this line to read "Reviewed by Adam Barth.", we can land it right now. > Source/WebCore/ChangeLog:12 > + Testing: existing tests (not all unforked yet) The "not all unforked yet" doesn't really add information. Could you change this line to say "Covered by existing tests (layout and instrumentation tests)."? 691448 8 157456 husky 2012-08-09 05:48:15 -0700 Created attachment 157456 Patch 691449 9 157443 husky 2012-08-09 05:48:33 -0700 Comment on attachment 157443 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157443&action=review >> Source/WebCore/ChangeLog:6 >> + Reviewed by NOBODY (OOPS!). > > Adam already r+'ed your patch :-). If you change this line to read "Reviewed by Adam Barth.", we can land it right now. Oh, cool! Although it seems like that would allow me to submit anything, which is surely dodgy, so I assume there's a catch... Done. >> Source/WebCore/ChangeLog:12 >> + Testing: existing tests (not all unforked yet) > > The "not all unforked yet" doesn't really add information. Could you change this line to say "Covered by existing tests (layout and instrumentation tests)."? Done. 691452 10 157456 peter 2012-08-09 05:49:55 -0700 Comment on attachment 157456 Patch The catch is that you need committer rights to set cq+ (or commit manually, of course), and we do trust our committers :-). 691635 11 157456 webkit.review.bot 2012-08-09 11:41:20 -0700 Comment on attachment 157456 Patch Clearing flags on attachment: 157456 Committed r125189: <http://trac.webkit.org/changeset/125189> 691637 12 webkit.review.bot 2012-08-09 11:41:25 -0700 All reviewed patches have been landed. Closing bug. 157245 2012-08-08 10:25:19 -0700 2012-08-09 05:21:26 -0700 Patch bug-92737-20120808182453.patch text/plain 1794 husky U3VidmVyc2lvbiBSZXZpc2lvbjogMTI0NDU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOTk4NWZhYmRhMGY0MWFl NGEzN2QwNjcyNWY2YzhjYjMxMGEwY2YyYi4uNWE4ZTE2NGRjZmJhYmNmN2Y1NGE4ZTdiYjU5ZWU0 MDUwZmJmNzZlYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA4LTA4ICBJYWlu IE1lcnJpY2sgIDxodXNreUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgRW5hYmxlIGtlcm5pbmcg b24gQW5kcm9pZAorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9OTI3MzcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBUaGlzICNpZmRlZiB3YXMgaW5jb3JyZWN0bHkgY29waWVkIGZyb20gb3VyIGZvcmtlZCBXZWJL aXQgcmVwb3NpdG9yeQorICAgICAgICB3aGlsZSB1bmZvcmtpbmcgb3VyIEFuZHJvaWQtc3BlY2lm aWMgY2hhbmdlcy4gS2VybmluZyB3b3JrcyB3aXRob3V0CisgICAgICAgIGFueSBrbm93biBwcm9i bGVtcy4KKworICAgICAgICBUZXN0aW5nOiBleGlzdGluZyBVSSB0ZXN0cyAobm90IGFsbCB1bmZv cmtlZCB5ZXQpCisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6ei9Db21wbGV4 VGV4dENvbnRyb2xsZXJIYXJmQnV6ei5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDb21wbGV4VGV4 dENvbnRyb2xsZXI6OkNvbXBsZXhUZXh0Q29udHJvbGxlcik6CisKIDIwMTItMDgtMDIgIEFsZXhh bmRlciBQYXZsb3YgIDxhcGF2bG92QGNocm9taXVtLm9yZz4KIAogICAgICAgICBXZWIgSW5zcGVj dG9yOiBNb3ZlIERPTSBicmVha3BvaW50cy1yZWxhdGVkIGNvbnRleHQgbWVudSBpdGVtcyBpbnRv IGEgc3VibWVudQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mv aGFyZmJ1enovQ29tcGxleFRleHRDb250cm9sbGVySGFyZkJ1enouY3BwIGIvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3JhcGhpY3MvaGFyZmJ1enovQ29tcGxleFRleHRDb250cm9sbGVySGFyZkJ1 enouY3BwCmluZGV4IDkxOTFkOWRmNjMyNDQzMmFkOGRiOWNlMDY0NjE2NWIwZWMzYmI1MTYuLjk0 MDFiMDEzNTk2NGZlYWJhNjM3OWU0MWJjYjBiNWU2ODhkOGNjMzQgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L0NvbXBsZXhUZXh0Q29udHJvbGxl ckhhcmZCdXp6LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJm YnV6ei9Db21wbGV4VGV4dENvbnRyb2xsZXJIYXJmQnV6ei5jcHAKQEAgLTgwLDExICs4MCw2IEBA IENvbXBsZXhUZXh0Q29udHJvbGxlcjo6Q29tcGxleFRleHRDb250cm9sbGVyKGNvbnN0IEZvbnQq IGZvbnQsIGNvbnN0IFRleHRSdW4mIHJ1CiAgICAgbV9pdGVtLnN0cmluZyA9IG1fbm9ybWFsaXpl ZEJ1ZmZlci5nZXQoKTsKICAgICBtX2l0ZW0uc3RyaW5nTGVuZ3RoID0gbV9ub3JtYWxpemVkQnVm ZmVyTGVuZ3RoOwogCi0jaWYgT1MoQU5EUk9JRCkKLSAgICAvLyBLZXJuaW5nIGRvZXMgbm90IGN1 cnJlbnRseSB3b3JrIG9uIEFuZHJvaWQuCi0gICAgbV9pdGVtLnNoYXBlckZsYWdzID0gSEJfU2hh cGVyRmxhZ19Ob0tlcm5pbmc7Ci0jZW5kaWYKLQogICAgIHJlc2V0KHN0YXJ0aW5nWCk7CiAgICAg bV9zdGFydGluZ1kgPSBzdGFydGluZ1k7CiAK 157443 2012-08-09 05:21:32 -0700 2012-08-09 05:48:33 -0700 Patch bug-92737-20120809132105.patch text/plain 1802 husky U3VidmVyc2lvbiBSZXZpc2lvbjogMTI0NDU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOTk4NWZhYmRhMGY0MWFl NGEzN2QwNjcyNWY2YzhjYjMxMGEwY2YyYi4uMzI0ZTY5YWZhYzljZjExOTViZTU0ZTBjOGUxMzBk MGRiNTUxYjc5NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA4LTA4ICBJYWlu IE1lcnJpY2sgIDxodXNreUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW2Nocm9taXVtXSBFbmFi bGUga2VybmluZyBvbiBBbmRyb2lkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD05MjczNworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFRoaXMgI2lmZGVmIHdhcyBpbmNvcnJlY3RseSBjb3BpZWQgZnJvbSBvdXIg Zm9ya2VkIFdlYktpdCByZXBvc2l0b3J5CisgICAgICAgIHdoaWxlIHVuZm9ya2luZyBvdXIgQW5k cm9pZC1zcGVjaWZpYyBjaGFuZ2VzLiBLZXJuaW5nIHdvcmtzIHdpdGhvdXQKKyAgICAgICAgYW55 IGtub3duIHByb2JsZW1zLgorCisgICAgICAgIFRlc3Rpbmc6IGV4aXN0aW5nIHRlc3RzIChub3Qg YWxsIHVuZm9ya2VkIHlldCkKKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6 L0NvbXBsZXhUZXh0Q29udHJvbGxlckhhcmZCdXp6LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkNv bXBsZXhUZXh0Q29udHJvbGxlcjo6Q29tcGxleFRleHRDb250cm9sbGVyKToKKwogMjAxMi0wOC0w MiAgQWxleGFuZGVyIFBhdmxvdiAgPGFwYXZsb3ZAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFdl YiBJbnNwZWN0b3I6IE1vdmUgRE9NIGJyZWFrcG9pbnRzLXJlbGF0ZWQgY29udGV4dCBtZW51IGl0 ZW1zIGludG8gYSBzdWJtZW51CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9oYXJmYnV6ei9Db21wbGV4VGV4dENvbnRyb2xsZXJIYXJmQnV6ei5jcHAgYi9Tb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6ei9Db21wbGV4VGV4dENvbnRyb2xs ZXJIYXJmQnV6ei5jcHAKaW5kZXggOTE5MWQ5ZGY2MzI0NDMyYWQ4ZGI5Y2UwNjQ2MTY1YjBlYzNi YjUxNi4uOTQwMWIwMTM1OTY0ZmVhYmE2Mzc5ZTQxYmNiMGI1ZTY4OGQ4Y2MzNCAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvaGFyZmJ1enovQ29tcGxleFRleHRD b250cm9sbGVySGFyZkJ1enouY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL2hhcmZidXp6L0NvbXBsZXhUZXh0Q29udHJvbGxlckhhcmZCdXp6LmNwcApAQCAtODAsMTEg KzgwLDYgQEAgQ29tcGxleFRleHRDb250cm9sbGVyOjpDb21wbGV4VGV4dENvbnRyb2xsZXIoY29u c3QgRm9udCogZm9udCwgY29uc3QgVGV4dFJ1biYgcnUKICAgICBtX2l0ZW0uc3RyaW5nID0gbV9u b3JtYWxpemVkQnVmZmVyLmdldCgpOwogICAgIG1faXRlbS5zdHJpbmdMZW5ndGggPSBtX25vcm1h bGl6ZWRCdWZmZXJMZW5ndGg7CiAKLSNpZiBPUyhBTkRST0lEKQotICAgIC8vIEtlcm5pbmcgZG9l cyBub3QgY3VycmVudGx5IHdvcmsgb24gQW5kcm9pZC4KLSAgICBtX2l0ZW0uc2hhcGVyRmxhZ3Mg PSBIQl9TaGFwZXJGbGFnX05vS2VybmluZzsKLSNlbmRpZgotCiAgICAgcmVzZXQoc3RhcnRpbmdY KTsKICAgICBtX3N0YXJ0aW5nWSA9IHN0YXJ0aW5nWTsKIAo= 157456 2012-08-09 05:48:15 -0700 2012-08-09 11:41:20 -0700 Patch bug-92737-20120809134748.patch text/plain 1813 husky U3VidmVyc2lvbiBSZXZpc2lvbjogMTI0NDU4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOTk4NWZhYmRhMGY0MWFl NGEzN2QwNjcyNWY2YzhjYjMxMGEwY2YyYi4uOWI4ZWRkZTBjMTNhMTk3ODk4OWMwYzc4MjEwOGFl ZThlNmJmNjJmMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEyLTA4LTA4ICBJYWlu IE1lcnJpY2sgIDxodXNreUBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgW2Nocm9taXVtXSBFbmFi bGUga2VybmluZyBvbiBBbmRyb2lkCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD05MjczNworCisgICAgICAgIFJldmlld2VkIGJ5IEFkYW0gQmFydGguCisK KyAgICAgICAgVGhpcyAjaWZkZWYgd2FzIGluY29ycmVjdGx5IGNvcGllZCBmcm9tIG91ciBmb3Jr ZWQgV2ViS2l0IHJlcG9zaXRvcnkKKyAgICAgICAgd2hpbGUgdW5mb3JraW5nIG91ciBBbmRyb2lk LXNwZWNpZmljIGNoYW5nZXMuIEtlcm5pbmcgd29ya3Mgd2l0aG91dAorICAgICAgICBhbnkga25v d24gcHJvYmxlbXMuCisKKyAgICAgICAgQ292ZXJlZCBieSBleGlzdGluZyB0ZXN0cyAobGF5b3V0 IGFuZCBpbnN0cnVtZW50YXRpb24gdGVzdHMpLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhp Y3MvaGFyZmJ1enovQ29tcGxleFRleHRDb250cm9sbGVySGFyZkJ1enouY3BwOgorICAgICAgICAo V2ViQ29yZTo6Q29tcGxleFRleHRDb250cm9sbGVyOjpDb21wbGV4VGV4dENvbnRyb2xsZXIpOgor CiAyMDEyLTA4LTAyICBBbGV4YW5kZXIgUGF2bG92ICA8YXBhdmxvdkBjaHJvbWl1bS5vcmc+CiAK ICAgICAgICAgV2ViIEluc3BlY3RvcjogTW92ZSBET00gYnJlYWtwb2ludHMtcmVsYXRlZCBjb250 ZXh0IG1lbnUgaXRlbXMgaW50byBhIHN1Ym1lbnUKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L0NvbXBsZXhUZXh0Q29udHJvbGxlckhhcmZCdXp6 LmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2hhcmZidXp6L0NvbXBsZXhU ZXh0Q29udHJvbGxlckhhcmZCdXp6LmNwcAppbmRleCA5MTkxZDlkZjYzMjQ0MzJhZDhkYjljZTA2 NDYxNjViMGVjM2JiNTE2Li45NDAxYjAxMzU5NjRmZWFiYTYzNzllNDFiY2IwYjVlNjg4ZDhjYzM0 IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9oYXJmYnV6ei9D b21wbGV4VGV4dENvbnRyb2xsZXJIYXJmQnV6ei5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vZ3JhcGhpY3MvaGFyZmJ1enovQ29tcGxleFRleHRDb250cm9sbGVySGFyZkJ1enouY3Bw CkBAIC04MCwxMSArODAsNiBAQCBDb21wbGV4VGV4dENvbnRyb2xsZXI6OkNvbXBsZXhUZXh0Q29u dHJvbGxlcihjb25zdCBGb250KiBmb250LCBjb25zdCBUZXh0UnVuJiBydQogICAgIG1faXRlbS5z dHJpbmcgPSBtX25vcm1hbGl6ZWRCdWZmZXIuZ2V0KCk7CiAgICAgbV9pdGVtLnN0cmluZ0xlbmd0 aCA9IG1fbm9ybWFsaXplZEJ1ZmZlckxlbmd0aDsKIAotI2lmIE9TKEFORFJPSUQpCi0gICAgLy8g S2VybmluZyBkb2VzIG5vdCBjdXJyZW50bHkgd29yayBvbiBBbmRyb2lkLgotICAgIG1faXRlbS5z aGFwZXJGbGFncyA9IEhCX1NoYXBlckZsYWdfTm9LZXJuaW5nOwotI2VuZGlmCi0KICAgICByZXNl dChzdGFydGluZ1gpOwogICAgIG1fc3RhcnRpbmdZID0gc3RhcnRpbmdZOwogCg==