45191 2010-09-03 11:57:45 -0700 [chromium] Fix complex text word spacing on Linux. 2010-09-13 11:47:39 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Other Linux RESOLVED FIXED P2 Normal --- 1 agl webkit-unassigned tony oldest_to_newest 274092 0 agl 2010-09-03 11:57:45 -0700 I broke complex text word spacing with r66689. I misnamed a variable in the original code |glyphIndex| when it was actually indexing code points. That meant that I compared it against the wrong limit when working around Harfbuzz issues and neatly disabled word spacing. 274094 1 66530 agl 2010-09-03 12:00:45 -0700 Created attachment 66530 Patch 274099 2 tony 2010-09-03 12:06:02 -0700 Can you also remove the line from test_expectations.txt where I marked the test as failing? 274111 3 66530 agl 2010-09-03 12:16:18 -0700 Comment on attachment 66530 Patch Will land on Monday (with the test-expectations changing included): landing on a Friday afternoon is tempting fate. 275032 4 agl 2010-09-07 07:54:04 -0700 r66882 275130 5 tony 2010-09-07 10:26:11 -0700 (In reply to comment #4) > r66882 Hmm, it still looks a bit off. It looks like the initial spacing is lost. Expected: http://trac.webkit.org/export/66888/trunk/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png Actual: http://build.chromium.org/buildbot/layout_test_results/webkit-rel-linux-webkit-org/results/layout-test-results/fast/text/atsui-spacing-features-actual.png 275132 6 tony 2010-09-07 10:26:37 -0700 Reopening 275135 7 agl 2010-09-07 10:29:22 -0700 (In reply to comment #5) > Hmm, it still looks a bit off. It looks like the initial spacing is lost. I checked by running the layout tests. However, I now imagine that the line in test_expectations.txt would have suppressed the failure, right? I'm not sure that either result is currently more 'right', but we should probably not change it for change's sake so I'll take another look. 275139 8 tony 2010-09-07 10:36:34 -0700 (In reply to comment #7) > (In reply to comment #5) > > Hmm, it still looks a bit off. It looks like the initial spacing is lost. > > I checked by running the layout tests. However, I now imagine that the line in test_expectations.txt would have suppressed the failure, right? Yes, if you ran the test before updating test_expectations.txt. > I'm not sure that either result is currently more 'right', but we should probably not change it for change's sake so I'll take another look. Thanks, Adam! Sorry this is such a yak shave . . . 278353 9 agl 2010-09-13 11:47:39 -0700 Rebaselined in r67400. I played with a few different things but nothing was clearly better. The new baseline doesn't include word-spacing at the right-hand-side of RTL text, which is better (I think). 66530 2010-09-03 12:00:45 -0700 2010-09-03 12:16:18 -0700 Patch bug-45191-20100903150044.patch text/plain 2072 agl ZGlmZiAtLWdpdCBhL1dlYkNvcmUvQ2hhbmdlTG9nIGIvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXgg MGUyNTkwOTM3NzhjNTVjYzgyZDI2Mjg4MDRhZjA0ZTY0MDY4OGFiZi4uODc0ZjdlNjJkYjc5Y2Ex ZWZjZDM0M2Q2Zjk5NGYyY2Q2OWNlMWZmNSAxMDA2NDQKLS0tIGEvV2ViQ29yZS9DaGFuZ2VMb2cK KysrIGIvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyMiBAQAorMjAxMC0wOS0wMyAgQWRh bSBMYW5nbGV5ICA8YWdsQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBbY2hyb21pdW1dIEZpeCBjb21wbGV4IHRleHQgd29yZCBz cGFjaW5nIG9uIExpbnV4LgorCisgICAgICAgIEkgYnJva2UgY29tcGxleCB0ZXh0IHdvcmQgc3Bh Y2luZyB3aXRoIHI2NjY4OS4gSSBtaXNuYW1lZCBhCisgICAgICAgIHZhcmlhYmxlIGluIHRoZSBv cmlnaW5hbCBjb2RlIHxnbHlwaEluZGV4fCB3aGVuIGl0IHdhcworICAgICAgICBhY3R1YWxseSBp bmRleGluZyBjb2RlIHBvaW50cy4gVGhhdCBtZWFudCB0aGF0IEkgY29tcGFyZWQgaXQKKyAgICAg ICAgYWdhaW5zdCB0aGUgd3JvbmcgbGltaXQgd2hlbiB3b3JraW5nIGFyb3VuZCBIYXJmYnV6eiBp c3N1ZXMKKyAgICAgICAgYW5kIG5lYXRseSBkaXNhYmxlZCB3b3JkIHNwYWNpbmcuCisKKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTQ1MTkxCisKKyAgICAg ICAgVGVzdDogZmFzdC90ZXh0L2F0c3VpLXNwYWNpbmctZmVhdHVyZXMuaHRtbAorCisgICAgICAg ICogcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vRm9udExpbnV4LmNwcDoKKyAgICAgICAgKFdl YkNvcmU6OlRleHRSdW5XYWxrZXI6OnNldEdseXBoWFBvc2l0aW9ucyk6CisKIDIwMTAtMDktMDMg IEJoYXJhdGh3YWFqIFNyaW5pdmFzYW4gIDxiaGFyYXRod2Fhai5zQGdtYWlsLmNvbT4KIAogICAg ICAgICBSZXZpZXdlZCBieSBYYW4gTG9wZXouCmRpZmYgLS1naXQgYS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL2Nocm9taXVtL0ZvbnRMaW51eC5jcHAgYi9XZWJDb3JlL3BsYXRmb3JtL2dyYXBo aWNzL2Nocm9taXVtL0ZvbnRMaW51eC5jcHAKaW5kZXggZTFjODZhMjU1M2UzNjdiYjMwYjEyNDRm MjgxOWMyNzIyM2RiMGRlNi4uM2E0MGNjNjg1MzdhODBmNTE2Y2JkMWJiYjQ5ODYxMmQ4YzUwMjIx MCAxMDA2NDQKLS0tIGEvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9jaHJvbWl1bS9Gb250TGlu dXguY3BwCisrKyBiL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vRm9udExpbnV4 LmNwcApAQCAtNTI2LDEyICs1MjYsMTEgQEAgcHJpdmF0ZToKICAgICAgICAgICAgIG1feFBvc2l0 aW9uc1tpXSA9IG1fb2Zmc2V0WCArIHBvc2l0aW9uICsgb2Zmc2V0WDsKIAogICAgICAgICAgICAg ZG91YmxlIGFkdmFuY2UgPSB0cnVuY2F0ZUZpeGVkUG9pbnRUb0ludGVnZXIobV9pdGVtLmFkdmFu Y2VzW2ldKTsKLSAgICAgICAgICAgIHVuc2lnbmVkIGdseXBoSW5kZXggPSBtX2l0ZW0uaXRlbS5w b3MgKyBsb2dDbHVzdGVyc0luZGV4OwogICAgICAgICAgICAgLy8gVGhlIGZpcnN0IGhhbGYgb2Yg dGhlIGNvbmp1Y3Rpb24gd29ya3MgYXJvdW5kIHRoZSBjYXNlIHdoZXJlCiAgICAgICAgICAgICAv LyBvdXRwdXQgZ2x5cGhzIGFyZW4ndCBhc3NvY2lhdGVkIHdpdGggYW55IGNvZGVwb2ludHMgYnkg dGhlCiAgICAgICAgICAgICAvLyBjbHVzdGVycyBsb2cuCi0gICAgICAgICAgICBpZiAoZ2x5cGhJ bmRleCA8IG1faXRlbS5udW1fZ2x5cGhzCi0gICAgICAgICAgICAgICAgJiYgaXNXb3JkQnJlYWso Z2x5cGhJbmRleCwgaXNSVEwpKSB7CisgICAgICAgICAgICBpZiAobG9nQ2x1c3RlcnNJbmRleCA8 IG1faXRlbS5pdGVtLmxlbmd0aAorICAgICAgICAgICAgICAgICYmIGlzV29yZEJyZWFrKG1faXRl bS5pdGVtLnBvcyArIGxvZ0NsdXN0ZXJzSW5kZXgsIGlzUlRMKSkgewogICAgICAgICAgICAgICAg IGFkdmFuY2UgKz0gbV93b3JkU3BhY2luZ0FkanVzdG1lbnQ7CiAKICAgICAgICAgICAgICAgICBp ZiAobV9wYWRkaW5nID4gMCkgewo=