Bug 66541 – [webkit-gtk] Run-time error while trying to run GtkLauncher with no video support.

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

[Nayan Kumar K]

We get following error while trying to launch GtkLauncher with video support disabled,

nayankk@qtc746-01:~/Sources/WebKit$ Tools/Scripts/run-launcher --gtk --debug 
Starting webkit launcher.
/home/nayankk/Sources/WebKit/WebKitBuild/Debug/Programs/GtkLauncher: symbol lookup error: /home/nayankk/Sources/WebKit/WebKitBuild/Debug/.libs/libwebkitgtk-1.0.so.0: undefined symbol: _ZN7WebCore14RenderThemeGtk25extraFullScreenStyleSheetEv

Reason for this being, in Source/WebCore/platform/gtk/RenderThemeGtk.h file, the function virtual String extraFullScreenStyleSheet(); is declared outside the ENABLE(VIDEO) guard, however it is defined in Source/WebCore/platform/gtk/RenderThemeGtk.cpp within ENABLE(VIDEO) guard. Ideally, definition of this function should lie within ENABLE(VIDEO) conditional compilation guard. 

This bug is raised to fix this error.

[Nayan Kumar K]

Created attachment 104469 [details]
Run time error fix

[WebKit Review Bot]

Attachment 104469 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Philippe Normand]

Comment on attachment 104469 [details]
Run time error fix

The Fullscreen feature doesn't depend on VIDEO, ideally it's RenderThemeGtk.cpp that needs to be fixed. For the ChangeLog you can use the prepare-ChangeLog script :)

[Nayan Kumar K]

(In reply to comment #3)
> (From update of attachment 104469 [details])
> The Fullscreen feature doesn't depend on VIDEO, ideally it's RenderThemeGtk.cpp that needs to be fixed.

On the first look, that was my guess too. However, when I looked at how this is being implemented in other ports, (Source/WebCore/rendering/RenderThemeChromiumMac.h,  Source/WebCore/rendering/RenderThemeMac.h and Source/WebCore/rendering/RenderThemeWin.h), they seems to be implementing it the other way! Given this, which way should we follow? (Does full screen here implies full screen video?)

 For the ChangeLog you can use the prepare-ChangeLog script :)
My emacs being the culprit here!. Will correct the patch and upload the new patch shortly!

[Nayan Kumar K]

Created attachment 104473 [details]
Updated patch, corrected indentation

[Philippe Normand]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 104469 [details] [details])
> > The Fullscreen feature doesn't depend on VIDEO, ideally it's RenderThemeGtk.cpp that needs to be fixed.
> 
> On the first look, that was my guess too. However, when I looked at how this is being implemented in other ports, (Source/WebCore/rendering/RenderThemeChromiumMac.h,  Source/WebCore/rendering/RenderThemeMac.h and Source/WebCore/rendering/RenderThemeWin.h), they seems to be implementing it the other way! Given this, which way should we follow? (Does full screen here implies full screen video?)
> 

Right let's do it like this then :)

[WebKit Review Bot]

Comment on attachment 104473 [details]
Updated patch, corrected indentation

Clearing flags on attachment: 104473

Committed r93391: <http://trac.webkit.org/changeset/93391>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.