Bug 131427 - [EFL] Change font path for DumpRenderTree and WebKitTestRunner
Summary: [EFL] Change font path for DumpRenderTree and WebKitTestRunner
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-04-08 22:27 PDT by Ryuan Choi
Modified: 2014-04-09 00:22 PDT (History)
8 users (show)

See Also:


Attachments
Patch (2.12 KB, patch)
2014-04-08 22:29 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (2.57 KB, patch)
2014-04-08 23:39 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2014-04-08 22:27:37 PDT
Since r166973, WebKit/Gtk changed font directory.
Comment 1 Ryuan Choi 2014-04-08 22:29:41 PDT
Created attachment 228935 [details]
Patch
Comment 2 Gyuyoung Kim 2014-04-08 22:33:54 PDT
Comment on attachment 228935 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228935&action=review

> Tools/DumpRenderTree/PlatformEfl.cmake:80
> +add_definitions(-DFONTS_CONF_DIR="${TOOLS_DIR}/WebKitTestRunner/gtk/fonts"

I think this change occurred because GTK port is deleting their wk1 port. However, we decide to keep wk1 port. It looks that now is time to have EFL own font directory path.
Comment 3 Ryuan Choi 2014-04-08 23:07:34 PDT
(In reply to comment #2)
> (From update of attachment 228935 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228935&action=review
> 
> > Tools/DumpRenderTree/PlatformEfl.cmake:80
> > +add_definitions(-DFONTS_CONF_DIR="${TOOLS_DIR}/WebKitTestRunner/gtk/fonts"
> 
> I think this change occurred because GTK port is deleting their wk1 port. However, we decide to keep wk1 port. It looks that now is time to have EFL own font directory path.

Yes, after removed GtKLauncher, fonts are just moved to WebKitTestRunner/gtk/fonts.
I don't think that it is good to copy them to efl specific folder.
And we don't have good fonts for efl port yet.

One solution is moving fonts to better place but I don't have idea.
Comment 4 Martin Robinson 2014-04-08 23:12:17 PDT
Comment on attachment 228935 [details]
Patch

Sorry about the breakage! I had no idea that EFL used our fonts directory.
Comment 5 Gyuyoung Kim 2014-04-08 23:19:48 PDT
Comment on attachment 228935 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=228935&action=review

> Tools/DumpRenderTree/PlatformEfl.cmake:78
>  # FIXME: DOWNLOADED_FONTS_DIR should not hardcode the directory

BTW, I think we can delete this FIXME: comment. Bug 81475 was closed.

>>> Tools/DumpRenderTree/PlatformEfl.cmake:80
>>> +add_definitions(-DFONTS_CONF_DIR="${TOOLS_DIR}/WebKitTestRunner/gtk/fonts"
>> 
>> I think this change occurred because GTK port is deleting their wk1 port. However, we decide to keep wk1 port. It looks that now is time to have EFL own font directory path.
> 
> Yes, after removed GtKLauncher, fonts are just moved to WebKitTestRunner/gtk/fonts.
> I don't think that it is good to copy them to efl specific folder.
> And we don't have good fonts for efl port yet.
> 
> One solution is moving fonts to better place but I don't have idea.

I think we need to add a comment. For example,

"FIXME: EFL port needs to have own test font and font configure instead of gtk test font in future"

> Tools/WebKitTestRunner/PlatformEfl.cmake:79
>  # FIXME: DOWNLOADED_FONTS_DIR should not hardcode the directory

ditto.
Comment 6 Gyuyoung Kim 2014-04-08 23:22:00 PDT
(In reply to comment #4)
> (From update of attachment 228935 [details])
> Sorry about the breakage! I had no idea that EFL used our fonts directory.

I wish EFL port has own test font in future. So, I would like to land this patch with a comment.
Comment 7 Ryuan Choi 2014-04-08 23:35:19 PDT
(In reply to comment #5)
> (From update of attachment 228935 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228935&action=review
> 
> > Tools/DumpRenderTree/PlatformEfl.cmake:78
> >  # FIXME: DOWNLOADED_FONTS_DIR should not hardcode the directory
> 
> BTW, I think we can delete this FIXME: comment. Bug 81475 was closed.
> 

Oh, right but without fixing "hardcode the directory". :(
Bug 81475 just fixed the issues about jhbuild.

I will remove url only and follow it in the another bug.

> >>> Tools/DumpRenderTree/PlatformEfl.cmake:80
> >>> +add_definitions(-DFONTS_CONF_DIR="${TOOLS_DIR}/WebKitTestRunner/gtk/fonts"
> >> 
> >> I think this change occurred because GTK port is deleting their wk1 port. However, we decide to keep wk1 port. It looks that now is time to have EFL own font directory path.
> > 
> > Yes, after removed GtKLauncher, fonts are just moved to WebKitTestRunner/gtk/fonts.
> > I don't think that it is good to copy them to efl specific folder.
> > And we don't have good fonts for efl port yet.
> > 
> > One solution is moving fonts to better place but I don't have idea.
> 
> I think we need to add a comment. For example,
> 
> "FIXME: EFL port needs to have own test font and font configure instead of gtk test font in future"
> 
> > Tools/WebKitTestRunner/PlatformEfl.cmake:79
> >  # FIXME: DOWNLOADED_FONTS_DIR should not hardcode the directory
> 
> ditto.

OK, I will.
Comment 8 Ryuan Choi 2014-04-08 23:39:30 PDT
Created attachment 228946 [details]
Patch
Comment 9 WebKit Commit Bot 2014-04-09 00:22:42 PDT
Comment on attachment 228946 [details]
Patch

Clearing flags on attachment: 228946

Committed r167004: <http://trac.webkit.org/changeset/167004>
Comment 10 WebKit Commit Bot 2014-04-09 00:22:49 PDT
All reviewed patches have been landed.  Closing bug.