27988 2009-08-04 10:08:06 -0700 word-spacing style seems not to be honored 2009-08-07 17:54:28 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) PC Linux RESOLVED FIXED http://www.w3.org/Style/CSS/Test/CSS1/current/sec541.htm P2 Normal --- 0 carol webkit-unassigned abarth eric hausmann kenneth laszlo.gombos oldest_to_newest 137038 0 carol 2009-08-04 10:08:06 -0700 Apparently the word-spacing style is ignored. 137149 1 mrowe 2009-08-04 15:22:05 -0700 With which port of WebKit? The page renders as expected on Mac OS X. 137309 2 carol 2009-08-05 08:36:55 -0700 The bug is reported against the Qt Linux port and apparently is due to a lack of implementation of custom text rendering. The drawComplexText method ignores most flags except for shadow and justification and renders the text using the default spacing. 137624 3 carol 2009-08-06 12:32:43 -0700 I found what the problem is: The Qt Font supports letter-spacing and word-spacing features, but the options on the Qt specific font are not updated from the members of the platform independent Font class. I have a fix already for this bug, but I still have to prepare a proper patch and provide a LayoutTest that would be fixed by the patch. 137974 4 34293 carol 2009-08-07 10:47:17 -0700 Created attachment 34293 Proposed patch Fixed QtWebkit behavior by applying the options to QFont when it is returned via Font::font(). 137991 5 34293 eric 2009-08-07 11:19:19 -0700 Comment on attachment 34293 Proposed patch Tabs. Why is this the right place? Why shouldn't these arguments be passed to getQtFont()? Why does Qt still use a different font architecture than the rest of the WebCore's ports? r- for the tabs. 138027 6 34311 carol 2009-08-07 12:02:22 -0700 Created attachment 34311 Fixed indent size. Fixed indent size per guidelines. The reason why I chose this fix is because I wanted to minimize differences between the Qt implementation and the other implementations. On most platforms letter spacing and word spacing is not handled like on Qt by the platform font; hence most of the platform independent infrastructure is set up for specifying platform fonts independent of word and letter spacing. Furthermore the letter and word spacing attributes can be set on the platform independent Font class via accessors that do not notify anybody such that the platform dependent dependent Font framework is unaware of changes to those properties hence the items in various caches cannot be updated. The getQtFont function is only called from the font() function that I changed. If moving my code to that function and adding to the function the two required extra parameters makes everybody happy, I will do that but I did not see the extra value (perhaps I do not understand the architecture enough). I tried to keep SimpleFontData as consistent as possible between platforms. I hate variations in interface based on platform, and I did not want to reinforce the behavior already present in SimpleFontData. 138148 7 eric 2009-08-07 14:25:08 -0700 (In reply to comment #6) > Created an attachment (id=34311) [details] > Fixed indent size. > > Fixed indent size per guidelines. In general using spacing in your comments will help make them easier to read. Extra lines between paragraphs will help my old eyes. ;) 138150 8 34311 eric 2009-08-07 14:26:27 -0700 Comment on attachment 34311 Fixed indent size. I'm OK with this change, but I would ask that you get Hyatt to spend 2 seconds looking at this. He can be found in #webkit as dhyatt or hyatt. 138177 9 34311 hyatt 2009-08-07 15:00:31 -0700 Comment on attachment 34311 Fixed indent size. r=me 138241 10 34311 abarth 2009-08-07 17:54:23 -0700 Comment on attachment 34311 Fixed indent size. Clearing review flag on attachment: 34311 Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebCore/ChangeLog M WebCore/platform/graphics/qt/FontQt.cpp Committed r46936 M WebCore/ChangeLog M WebCore/platform/graphics/qt/FontQt.cpp r46936 = 66a8a336fd67a420c6bb06b5976ed7616c18d683 (trunk) No changes between current HEAD and refs/remotes/trunk Resetting to the latest refs/remotes/trunk http://trac.webkit.org/changeset/46936 138242 11 abarth 2009-08-07 17:54:28 -0700 All reviewed patches have been landed. Closing bug. 34293 2009-08-07 10:47:17 -0700 2009-08-07 12:02:22 -0700 Proposed patch patchFor27988.txt text/plain 1640 carol SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0Njg1NCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjUgQEAKKzIwMDktMDgtMDYgIFN6YWJvIENhcm9sICA8Y2Fyb2wuc3phYm9Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFF0V2Via2l0IGlnbm9yZXMgbGV0dGVyLXNwYWNpbmcgYW5kIHdvcmQtc3BhY2luZyBzdHlsZXMu CisgICAgICAgIFF0V2Via2l0IGhhbmRsZXMgY3VzdG9tIGxldHRlci1zcGFjaW5nIGFuZCB3b3Jk LXNwYWNpbmcgaW4gdGhlIG5hdGl2ZSBRRm9udAorICAgICAgICBjbGFzcywgYnV0IHRoZSB2YWx1 ZXMgZm9yIHRoZXNlIG9wdGlvbnMgYXJlIG5vdCBub3QgcGFzc2VkIG9uIGZyb20KKyAgICAgICAg dGhlIHBsYXRmb3JtIGluZGVwZW5kZW50IHBhcnQgb2YgdGhlIEZvbnQgY2xhc3MuCisgICAgICAg IEZpeGVkIFF0V2Via2l0IGJlaGF2aW9yIGJ5IGFwcGx5aW5nIHRoZSBvcHRpb25zIHRvIFFGb250 IHdoZW4gaXQgaXMKKyAgICAgICAgcmV0dXJuZWQgdmlhIEZvbnQ6OmZvbnQoKQorCisgICAgICAg IGh0dHA6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI3OTg4CisKKyAgICAgICAg TGF5b3V0VGVzdHM6CisgICAgICAgIGZhc3QvY3NzL3dvcmQtc3BhY2UtZXh0cmEuaHRtbAorICAg ICAgICBjc3MyLjEvdDE2MDQtYzU0Mi1sZXR0ZXItc3AtMDAtYi1hLmh0bWwKKyAgICAgICAgY3Nz Mi4xL3QxNjA0LWM1NDEtd29yZC1zcC0wMS1iLWEuaHRtbAorICAgICAgICBjc3MxL3RleHRfcHJv cGVydGllcy93b3JkX3NwYWNpbmcuaHRtbAorICAgICAgICAKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy9xdC9Gb250UXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Rm9udDo6Zm9udCk6CisK IDIwMDktMDgtMDYgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIEZp eCBNYWMgTGVvcGFyZCBkZWJ1ZyBidWlsZC4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvcXQvRm9udFF0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L3F0L0ZvbnRRdC5jcHAJKHJldmlzaW9uIDQ2ODUzKQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9xdC9Gb250UXQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yMjEsNyArMjIxLDEwIEBAIEZs b2F0UmVjdCBGb250OjpzZWxlY3Rpb25SZWN0Rm9yQ29tcGxleFQKIAogUUZvbnQgRm9udDo6Zm9u dCgpIGNvbnN0CiB7Ci0gICAgcmV0dXJuIHByaW1hcnlGb250KCktPmdldFF0Rm9udCgpOworICBR Rm9udCBmID0gcHJpbWFyeUZvbnQoKS0+Z2V0UXRGb250KCk7CisgIGYuc2V0TGV0dGVyU3BhY2lu ZyhRRm9udDo6QWJzb2x1dGVTcGFjaW5nLCBtX2xldHRlclNwYWNpbmcpOworICBmLnNldFdvcmRT cGFjaW5nKG1fd29yZFNwYWNpbmcpOworICByZXR1cm4gZjsKIH0KIAogfQo= 34311 2009-08-07 12:02:22 -0700 2009-08-07 17:54:23 -0700 Fixed indent size. patchFor27988.txt text/plain 1648 carol SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0Njg1NCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMjUgQEAKKzIwMDktMDgtMDYgIFN6YWJvIENhcm9sICA8Y2Fyb2wuc3phYm9Abm9r aWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IFF0V2Via2l0IGlnbm9yZXMgbGV0dGVyLXNwYWNpbmcgYW5kIHdvcmQtc3BhY2luZyBzdHlsZXMu CisgICAgICAgIFF0V2Via2l0IGhhbmRsZXMgY3VzdG9tIGxldHRlci1zcGFjaW5nIGFuZCB3b3Jk LXNwYWNpbmcgaW4gdGhlIG5hdGl2ZSBRRm9udAorICAgICAgICBjbGFzcywgYnV0IHRoZSB2YWx1 ZXMgZm9yIHRoZXNlIG9wdGlvbnMgYXJlIG5vdCBub3QgcGFzc2VkIG9uIGZyb20KKyAgICAgICAg dGhlIHBsYXRmb3JtIGluZGVwZW5kZW50IHBhcnQgb2YgdGhlIEZvbnQgY2xhc3MuCisgICAgICAg IEZpeGVkIFF0V2Via2l0IGJlaGF2aW9yIGJ5IGFwcGx5aW5nIHRoZSBvcHRpb25zIHRvIFFGb250 IHdoZW4gaXQgaXMKKyAgICAgICAgcmV0dXJuZWQgdmlhIEZvbnQ6OmZvbnQoKQorCisgICAgICAg IGh0dHA6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTI3OTg4CisKKyAgICAgICAg TGF5b3V0VGVzdHM6CisgICAgICAgIGZhc3QvY3NzL3dvcmQtc3BhY2UtZXh0cmEuaHRtbAorICAg ICAgICBjc3MyLjEvdDE2MDQtYzU0Mi1sZXR0ZXItc3AtMDAtYi1hLmh0bWwKKyAgICAgICAgY3Nz Mi4xL3QxNjA0LWM1NDEtd29yZC1zcC0wMS1iLWEuaHRtbAorICAgICAgICBjc3MxL3RleHRfcHJv cGVydGllcy93b3JkX3NwYWNpbmcuaHRtbAorICAgICAgICAKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy9xdC9Gb250UXQuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Rm9udDo6Zm9udCk6CisK IDIwMDktMDgtMDYgIERhcmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIEZp eCBNYWMgTGVvcGFyZCBkZWJ1ZyBidWlsZC4KSW5kZXg6IFdlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvcXQvRm9udFF0LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNz L3F0L0ZvbnRRdC5jcHAJKHJldmlzaW9uIDQ2ODUzKQorKysgV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9xdC9Gb250UXQuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yMjEsNyArMjIxLDEwIEBAIEZs b2F0UmVjdCBGb250OjpzZWxlY3Rpb25SZWN0Rm9yQ29tcGxleFQKIAogUUZvbnQgRm9udDo6Zm9u dCgpIGNvbnN0CiB7Ci0gICAgcmV0dXJuIHByaW1hcnlGb250KCktPmdldFF0Rm9udCgpOworICAg IFFGb250IGYgPSBwcmltYXJ5Rm9udCgpLT5nZXRRdEZvbnQoKTsKKyAgICBmLnNldExldHRlclNw YWNpbmcoUUZvbnQ6OkFic29sdXRlU3BhY2luZywgbV9sZXR0ZXJTcGFjaW5nKTsKKyAgICBmLnNl dFdvcmRTcGFjaW5nKG1fd29yZFNwYWNpbmcpOworICAgIHJldHVybiBmOwogfQogCiB9Cg==