| Summary: | word-spacing style seems not to be honored | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Carol Szabo <carol> | ||||||
| Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, eric, hausmann, kenneth, laszlo.gombos | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| URL: | http://www.w3.org/Style/CSS/Test/CSS1/current/sec541.htm | ||||||||
| Attachments: |
|
||||||||
|
Description
Carol Szabo
2009-08-04 10:08:06 PDT
With which port of WebKit? The page renders as expected on Mac OS X. 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. 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. Created attachment 34293 [details]
Proposed patch
Fixed QtWebkit behavior by applying the options to QFont when it is returned via Font::font().
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.
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.
(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 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 on attachment 34311 [details]
Fixed indent size.
r=me
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 All reviewed patches have been landed. Closing bug. |