Bug 27988 - word-spacing style seems not to be honored
Summary: word-spacing style seems not to be honored
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL: http://www.w3.org/Style/CSS/Test/CSS1...
Depends on:
Reported: 2009-08-04 10:08 PDT by Carol Szabo
Modified: 2009-08-07 17:54 PDT (History)
5 users (show)

See Also:

Proposed patch (1.60 KB, patch)
2009-08-07 10:47 PDT, Carol Szabo
eric: review-
Details | Formatted Diff | Diff
Fixed indent size. (1.61 KB, patch)
2009-08-07 12:02 PDT, Carol Szabo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carol Szabo 2009-08-04 10:08:06 PDT
Apparently the word-spacing style is ignored.
Comment 1 Mark Rowe (bdash) 2009-08-04 15:22:05 PDT
With which port of WebKit?  The page renders as expected on Mac OS X.
Comment 2 Carol Szabo 2009-08-05 08:36:55 PDT
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.
Comment 3 Carol Szabo 2009-08-06 12:32:43 PDT
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.
Comment 4 Carol Szabo 2009-08-07 10:47:17 PDT
Created attachment 34293 [details]
Proposed patch

Fixed QtWebkit behavior by applying the options to QFont when it is returned via Font::font().
Comment 5 Eric Seidel (no email) 2009-08-07 11:19:19 PDT
Comment on attachment 34293 [details]
Proposed patch


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.
Comment 6 Carol Szabo 2009-08-07 12:02:22 PDT
Created attachment 34311 [details]
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.
Comment 7 Eric Seidel (no email) 2009-08-07 14:25:08 PDT
(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. ;)
Comment 8 Eric Seidel (no email) 2009-08-07 14:26:27 PDT
Comment on attachment 34311 [details]
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.
Comment 9 Dave Hyatt 2009-08-07 15:00:31 PDT
Comment on attachment 34311 [details]
Fixed indent size.

Comment 10 Adam Barth 2009-08-07 17:54:23 PDT
Comment on attachment 34311 [details]
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
Comment 11 Adam Barth 2009-08-07 17:54:28 PDT
All reviewed patches have been landed.  Closing bug.