Bug 66541

Summary: [webkit-gtk] Run-time error while trying to run GtkLauncher with no video support.
Product: WebKit Reporter: Nayan Kumar K <nayankk>
Component: WebKitGTKAssignee: Nayan Kumar K <nayankk>
Status: RESOLVED FIXED    
Severity: Normal CC: mrobinson, pnormand, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Run time error fix
pnormand: review-
Updated patch, corrected indentation none

Description Nayan Kumar K 2011-08-18 23:18:58 PDT
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.
Comment 1 Nayan Kumar K 2011-08-19 00:02:46 PDT
Created attachment 104469 [details]
Run time error fix
Comment 2 WebKit Review Bot 2011-08-19 00:05:11 PDT
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.
Comment 3 Philippe Normand 2011-08-19 00:12:22 PDT
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 :)
Comment 4 Nayan Kumar K 2011-08-19 00:21:46 PDT
(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!
Comment 5 Nayan Kumar K 2011-08-19 00:44:33 PDT
Created attachment 104473 [details]
Updated patch, corrected indentation
Comment 6 Philippe Normand 2011-08-19 00:56:08 PDT
(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 :)
Comment 7 WebKit Review Bot 2011-08-19 01:58:40 PDT
Comment on attachment 104473 [details]
Updated patch, corrected indentation

Clearing flags on attachment: 104473

Committed r93391: <http://trac.webkit.org/changeset/93391>
Comment 8 WebKit Review Bot 2011-08-19 01:58:44 PDT
All reviewed patches have been landed.  Closing bug.