Bug 27988 – word-spacing style seems not to be honored

• Component: New Bugs
• Status: RESOLVED FIXED

[Carol Szabo]

Apparently the word-spacing style is ignored.

[Mark Rowe (bdash)]

With which port of WebKit?  The page renders as expected on Mac OS X.

[Carol Szabo]

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.

[Carol Szabo]

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.

[Carol Szabo]

Created attachment 34293 [details]
Proposed patch

Fixed QtWebkit behavior by applying the options to QFont when it is returned via Font::font().

[Eric Seidel (no email)]

Comment on attachment 34293 [details]
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.

[Carol Szabo]

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.

[Eric Seidel (no email)]

(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. ;)

[Eric Seidel (no email)]

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.

[Dave Hyatt]

Comment on attachment 34311 [details]
Fixed indent size.

r=me

[Adam Barth]

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
http://trac.webkit.org/changeset/46936

[Adam Barth]

All reviewed patches have been landed.  Closing bug.