Bug 112704

Summary: [Qt] Enable kerning by default
Product: WebKit Reporter: Allan Sandfeld Jensen <allan.jensen>
Component: WebKit QtAssignee: Allan Sandfeld Jensen <allan.jensen>
Status: RESOLVED FIXED    
Severity: Normal CC: jturcotte, pierre.rossi
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 106013    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch jturcotte: review+

Allan Sandfeld Jensen
Reported 2013-03-19 09:09:53 PDT
Since kerning is now possible in the fast font-path, we should enable kerning by default as we do in the rest of Qt.
Attachments
Patch (5.47 KB, patch)
2013-03-19 10:46 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (5.57 KB, patch)
2013-03-20 03:56 PDT, Allan Sandfeld Jensen 		no flags
DetailsFormatted DiffDiff
Patch (5.56 KB, patch)
2013-03-20 04:15 PDT, Allan Sandfeld Jensen 		jturcotte: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Allan Sandfeld Jensen
Comment 1 2013-03-19 10:46:13 PDT
Created attachment 193858 [details] Patch
Jocelyn Turcotte
Comment 2 2013-03-20 03:47:54 PDT
Comment on attachment 193858 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193858&action=review > Source/WebKit/qt/WebCoreSupport/InitWebCoreQt.cpp:77 > + WebCore::Font::setDefaultTypesettingFeatures(WebCore::Kerning); Comment please :)
Allan Sandfeld Jensen
Comment 3 2013-03-20 03:56:36 PDT
Created attachment 194014 [details] Patch
Jocelyn Turcotte
Comment 4 2013-03-20 04:00:09 PDT
Comment on attachment 194014 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194014&action=review > Tools/DumpRenderTree/qt/TestRunnerQt.cpp:91 > + DumpRenderTreeSupportQt::setDefaultTypesettingFeatures(0); This is a bit stretched, passing an int knowing that the caller will call it with 0 and won't have to include WebCore headers. DumpRenderTreeSupportQt doesn't seem to expose any other WebCore type that way, so maybe this should be exposed as a "disableAll" method or as individual bool settings for kerning and ligatures independently?
Jocelyn Turcotte
Comment 5 2013-03-20 04:05:40 PDT
Pierre, I'd also like to know if this looks fine to you, could you have a look?
Allan Sandfeld Jensen
Comment 6 2013-03-20 04:08:56 PDT
(In reply to comment #4) > (From update of attachment 194014 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=194014&action=review > > > Tools/DumpRenderTree/qt/TestRunnerQt.cpp:91 > > + DumpRenderTreeSupportQt::setDefaultTypesettingFeatures(0); > > This is a bit stretched, passing an int knowing that the caller will call it with 0 and won't have to include WebCore headers. > DumpRenderTreeSupportQt doesn't seem to expose any other WebCore type that way, so maybe this should be exposed as a "disableAll" method or as individual bool settings for kerning and ligatures independently? I will make it disable all. This is all we really need since the layout expectations simply expects how it used to be, with all off by default.
Allan Sandfeld Jensen
Comment 7 2013-03-20 04:15:42 PDT
Created attachment 194015 [details] Patch
Jocelyn Turcotte
Comment 8 2013-03-20 06:44:49 PDT
Comment on attachment 194015 [details] Patch 14:32 < elproxy> LGTM. r=me
Allan Sandfeld Jensen
Comment 9 2013-03-20 07:21:18 PDT
Committed r146340: <http://trac.webkit.org/changeset/146340>
Note You need to log in before you can comment on or make changes to this bug.