Bug 177938 – [GTK] 2 text tests failing since r222838

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Miguel Gomez]

fast/text/woff2-totalsfntsize.html [ ImageOnlyFailure ]
fast/text/woff2.html [ ImageOnlyFailure ]

[Tomas Popela]

OK, so the problem was that in OptionGTK.cmake we had:

set(USE_WOFF2 ON)

but in should use:

SET_AND_EXPOSE_TO_BUILD(USE_WOFF2 ON)

but the tests should be fixed now with bug 177804 (r222907):

01:36:41.601 16255 [22052/46035] fast/text/woff2-totalsfntsize.html passed unexpectedly
01:36:41.600 16368 worker/2 fast/text/woff2-totalsfntsize.html passed
01:36:41.949 16255 [22061/46035] fast/text/woff2.html passed unexpectedly
01:36:41.949 16368 worker/2 fast/text/woff2.html passed

It will mark them as passing..

[Tomas Popela]

Created attachment 323005 [details]
Patch

[Michael Catanzaro]

(In reply to Tomas Popela from comment #1)
> OK, so the problem was that in OptionGTK.cmake we had:
> 
> set(USE_WOFF2 ON)
> 
> but in should use:
> 
> SET_AND_EXPOSE_TO_BUILD(USE_WOFF2 ON)

Is this fixed yet...?

[Tomas Popela]

(In reply to Michael Catanzaro from comment #3)
> (In reply to Tomas Popela from comment #1)
> > OK, so the problem was that in OptionGTK.cmake we had:
> > 
> > set(USE_WOFF2 ON)
> > 
> > but in should use:
> > 
> > SET_AND_EXPOSE_TO_BUILD(USE_WOFF2 ON)
> 
> Is this fixed yet...?

It is, but differently.. now it is defined with WEBKIT_OPTION_DEFINE()..

[Tomas Popela]

Comment on attachment 323005 [details]
Patch

Clearing flags on attachment: 323005

Committed r222973: <http://trac.webkit.org/changeset/222973>

[Tomas Popela]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

What about the 2.18 branch?

And there's also USE_CAIRO and USE_XDGMIME that are set but not exposed to build. Hmmmmm.

[Tomas Popela]

(In reply to Michael Catanzaro from comment #7)
> What about the 2.18 branch?

That one needs to be fixed..
 
> And there's also USE_CAIRO and USE_XDGMIME that are set but not exposed to
> build. Hmmmmm.

Exactly.. Man you should really be on IRC (and especially on FreeNode) ;)..

[Michael Catanzaro]

We need to figure this out.

USE(WOFF2) needs fixed for 2.18.

And it's hard to believe that USE(CAIRO) is not working, but I'm not sure how.

[Tomas Popela]

(In reply to Michael Catanzaro from comment #9)
> We need to figure this out.
> 
> USE(WOFF2) needs fixed for 2.18.
> 

For 2.18 it will be fixed by backporting r222838 there together with:

diff --git a/Source/cmake/OptionsGTK.cmake b/Source/cmake/OptionsGTK.cmake
index 0caa7dce0b8..b84419fc581 100644
--- a/Source/cmake/OptionsGTK.cmake
+++ b/Source/cmake/OptionsGTK.cmake
@@ -56,7 +56,7 @@ WEBKIT_OPTION_BEGIN()

 include(GStreamerDefinitions)

-set(USE_CAIRO ON)
+SET_AND_EXPOSE_TO_BUILD(USE_CAIRO ON)
 set(USE_WOFF2 ON)
 set(USE_XDGMIME ON)
 SET_AND_EXPOSE_TO_BUILD(USE_GCRYPT TRUE)

> And it's hard to believe that USE(CAIRO) is not working, but I'm not sure
> how.

it is not as it is the same situation as here.. It needs to use the SET_AND_EXPOSE_TO_BUILD() and we have to remove its override from Platform.h

[Michael Catanzaro]

Let's leave USE_CAIRO alone for now... it's awkward that it's set separately in Platform.h and Options[GTK,WPE].cmake, but there is not currently any correctness issue and this will be a difficult cleanup, for another day. Not really sure how we want to handle it. It doesn't make much sense to me to move it from Platform.h to CMake while leaving all the other stuff defined next to it in Platform.h, but it's true that it's clearly needed in CMake as well. Maybe we should be moving a lot more into CMake.

I don't think r222838 is needed for 2.18, since we don't support disabling WOFF2 there. In fact, backporting that would introduce the regression, so it must not be backported.

So I think we're good here... sort of.