You may want to use InternalSettings class which lives in Source/WebCore/testing.

bashi, thanks for the pointer. It looks like InternalSettings is like an alternative to overridePreference. Do you know what are the reasons to use one or the other? It seems great if it works on all ports automatically. Then the DRT for each port won't have to implement per-script font settings.

(In reply to comment #2) > bashi, thanks for the pointer. It looks like InternalSettings is like an alternative to overridePreference. Do you know what are the reasons to use one or the other? It seems great if it works on all ports automatically. Then the DRT for each port won't have to implement per-script font settings. If my understanding is correct, it would be better to use InternalSettings than DRT. It should work all ports. Morrita-san, please correct me if I'm wrong.

Created attachment 128384 [details] Patch

Hi Morrita-san, what do you think of this approach? It's not ready to commit yet... I get a linker error when I try to build on Mac and still have to update the ChangeLog.

Comment on attachment 128384 [details] Patch Attachment 128384 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11602093

Comment on attachment 128384 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=128384&action=review > Source/WebCore/ChangeLog:3 > + [DRT] Support per-script font settings in overridePreference nit: you should remove [DRT] prefix because there is no DRT related stuff. Please add description of this change. > Source/WebCore/ChangeLog:8 > + No new tests. (OOPS!) Please explain why tests are not necessary. > LayoutTests/ChangeLog:5 > + Please add description.

(In reply to comment #5) > Hi Morrita-san, what do you think of this approach? It's not ready to commit yet... I get a linker error when I try to build on Mac and still have to update the ChangeLog. You may need to add symbols to WebCore.exp.in and symbols.filter like http://trac.webkit.org/changeset/106146

Created attachment 128404 [details] Patch

bashi@ thanks for the comments. It builds on Mac now.

Comment on attachment 128404 [details] Patch Attachment 128404 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11610117

(In reply to comment #11) > (From update of attachment 128404 [details]) > Attachment 128404 [details] did not pass gtk-ews (gtk): > Output: http://queues.webkit.org/results/11610117 Looks like you may also need to export Settings::setXXXFontFamily() functions. ./.libs/libWebCoreInternals.a(./.libs/../Source/WebCore/testing/.libs/libWebCoreInternals_la-InternalSettings.o):InternalSettings.cpp:function WebCore::setStandardFontFamilyWrapper(WebCore::Settings*, WTF::String const&, UScriptCode): error: undefined reference to 'WebCore::Settings::setStandardFontFamily(WTF::AtomicString const&, UScriptCode)'

Could a GTK expert please advise me on what symbols to add to WebCore.exp.in or symbols.filter? I'm having trouble setting up an environment to build WebKitGTK+ myself. Morrita-san, what do you think the overall approach of adding this to InternalSettings?

(In reply to comment #13) > Could a GTK expert please advise me on what symbols to add to WebCore.exp.in or symbols.filter? I'm having trouble setting up an environment to build WebKitGTK+ myself. > > Morrita-san, what do you think the overall approach of adding this to InternalSettings? I'm not really sure about it, but looks like these symbols should be added in symbols.filter. _ZNK7WebCore8Settings15fixedFontFamilyE11UScriptCode _ZNK7WebCore8Settings15serifFontFamilyE11UScriptCode _ZNK7WebCore8Settings17cursiveFontFamilyE11UScriptCode _ZNK7WebCore8Settings17fantasyFontFamilyE11UScriptCode _ZNK7WebCore8Settings18standardFontFamilyE11UScriptCode _ZNK7WebCore8Settings19sansSerifFontFamilyE11UScriptCode _ZNK7WebCore8Settings20pictographFontFamilyE11UScriptCode

(In reply to comment #14) > _ZNK7WebCore8Settings15fixedFontFamilyE11UScriptCode > _ZNK7WebCore8Settings15serifFontFamilyE11UScriptCode > _ZNK7WebCore8Settings17cursiveFontFamilyE11UScriptCode > _ZNK7WebCore8Settings17fantasyFontFamilyE11UScriptCode > _ZNK7WebCore8Settings18standardFontFamilyE11UScriptCode > _ZNK7WebCore8Settings19sansSerifFontFamilyE11UScriptCode > _ZNK7WebCore8Settings20pictographFontFamilyE11UScriptCode I made mistake. These should be: _ZN7WebCore8Settings18setFixedFontFamilyERKN3WTF12AtomicStringE11UScriptCode _ZN7WebCore8Settings18setSerifFontFamilyERKN3WTF12AtomicStringE11UScriptCode _ZN7WebCore8Settings20setCursiveFontFamilyERKN3WTF12AtomicStringE11UScriptCode _ZN7WebCore8Settings20setFantasyFontFamilyERKN3WTF12AtomicStringE11UScriptCode _ZN7WebCore8Settings21setStandardFontFamilyERKN3WTF12AtomicStringE11UScriptCode _ZN7WebCore8Settings22setSansSerifFontFamilyERKN3WTF12AtomicStringE11UScriptCode _ZN7WebCore8Settings23setPictographFontFamilyERKN3WTF12AtomicStringE11UScriptCode

Updating summary to what the patch is trying to do.

Created attachment 130930 [details] Patch

Created attachment 131303 [details] remove expectation pngs

Comment on attachment 131303 [details] remove expectation pngs Attachment 131303 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/11940796

Created attachment 131551 [details] add WebCore.exp.in back in

Comment on attachment 131551 [details] add WebCore.exp.in back in View in context: https://bugs.webkit.org/attachment.cgi?id=131551&action=review The approach looks good. added some nitpicky comments. > Source/WebCore/testing/InternalSettings.cpp:287 > + (*setter)(settings(), family, code); I think we can use a member function pointer here. http://stackoverflow.com/questions/5499155/c-member-function-pointer > Source/WebCore/testing/InternalSettings.h:33 > +#include <wtf/unicode/Unicode.h> Could you get rid of this include? I want the dependency to be as small as possible. > Source/WebCore/testing/InternalSettings.h:85 > + void setFontFamily(const String& family, const String& script, SetFontFamilyWrapper setter); For that purpose, this could be be s static function.

Created attachment 131802 [details] Patch

Comment on attachment 131551 [details] add WebCore.exp.in back in View in context: https://bugs.webkit.org/attachment.cgi?id=131551&action=review Thanks for the review. I've uploaded a new patch. >> Source/WebCore/testing/InternalSettings.cpp:287 >> + (*setter)(settings(), family, code); > > I think we can use a member function pointer here. > http://stackoverflow.com/questions/5499155/c-member-function-pointer Done. >> Source/WebCore/testing/InternalSettings.h:33 >> +#include <wtf/unicode/Unicode.h> > > Could you get rid of this include? I want the dependency to be as small as possible. Done. >> Source/WebCore/testing/InternalSettings.h:85 >> + void setFontFamily(const String& family, const String& script, SetFontFamilyWrapper setter); > > For that purpose, this could be be s static function. Done.

Comment on attachment 131802 [details] Patch Looks good. Please wait until bots become green.

Comment on attachment 131802 [details] Patch Rejecting attachment 131802 [details] from commit-queue. falken@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.

Comment on attachment 131802 [details] Patch Clearing flags on attachment: 131802 Committed r110808: <http://trac.webkit.org/changeset/110808>

All reviewed patches have been landed. Closing bug.