Bug 51755 - [GTK] Fork pieces of RenderThemeGtk that will differ for GTK+ 3
Summary: [GTK] Fork pieces of RenderThemeGtk that will differ for GTK+ 3
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 50820
  Show dependency treegraph
 
Reported: 2010-12-30 10:11 PST by Martin Robinson
Modified: 2010-12-30 17:06 PST (History)
1 user (show)

See Also:


Attachments
Split out methods that will differ (66.52 KB, patch)
2010-12-30 10:22 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch with Xan's suggestions (62.05 KB, patch)
2010-12-30 16:10 PST, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-12-30 10:11:24 PST
The first step to landing the new GTK+ style context port of RenderThemeGtk is to split out the pieces of RenderThemeGtk that will differ between versions into RenderThemeGtk2 and RenderThemeGtk3.
Comment 1 Martin Robinson 2010-12-30 10:22:47 PST
Created attachment 77687 [details]
Split out methods that will differ
Comment 2 WebKit Review Bot 2010-12-30 10:26:21 PST
Attachment 77687 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/GNUmakefile.am', u'WebCore/platform/gtk/RenderThemeGtk.cpp', u'WebCore/platform/gtk/RenderThemeGtk.h', u'WebCore/platform/gtk/RenderThemeGtk2.cpp', u'WebCore/platform/gtk/RenderThemeGtk3.cpp']" exit_code: 1
WebCore/platform/gtk/RenderThemeGtk2.cpp:164:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/RenderThemeGtk2.cpp:340:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/RenderThemeGtk3.cpp:164:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/RenderThemeGtk3.cpp:340:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Martin Robinson 2010-12-30 11:42:12 PST
(In reply to comment #2)
> Attachment 77687 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/GNUmakefile.am', u'WebCore/platform/gtk/RenderThemeGtk.cpp', u'WebCore/platform/gtk/RenderThemeGtk.h', u'WebCore/platform/gtk/RenderThemeGtk2.cpp', u'WebCore/platform/gtk/RenderThemeGtk3.cpp']" exit_code: 1
> WebCore/platform/gtk/RenderThemeGtk2.cpp:164:  Use 0 instead of NULL.  [readability/null] [5]
> WebCore/platform/gtk/RenderThemeGtk2.cpp:340:  Use 0 instead of NULL.  [readability/null] [5]
> WebCore/platform/gtk/RenderThemeGtk3.cpp:164:  Use 0 instead of NULL.  [readability/null] [5]
> WebCore/platform/gtk/RenderThemeGtk3.cpp:340:  Use 0 instead of NULL.  [readability/null] [5]
> Total errors found: 4 in 6 files
> 
> 
> If any of these errors are false positives, please file a bug against check-webkit-style.

This is a bug in check-webkit-style: https://bugs.webkit.org/show_bug.cgi?id=51758
Comment 4 Xan Lopez 2010-12-30 15:38:06 PST
Comment on attachment 77687 [details]
Split out methods that will differ

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

> WebCore/ChangeLog:105
> +

Either remove all that stuff or fill in the details?

> WebCore/platform/gtk/RenderThemeGtk.cpp:456
> +                   gtkTextDirection(renderObject->style()->direction()),

This is somewhat random :D (I believe in general adding extra newlines goes against the style guidelines?)

> WebCore/platform/gtk/RenderThemeGtk3.cpp:156
> +#endif

This is always going to be FALSE for GTK+ 3.x right?
Comment 5 Martin Robinson 2010-12-30 15:53:28 PST
Comment on attachment 77687 [details]
Split out methods that will differ

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

Thanks for the review!

>> WebCore/ChangeLog:105
>> +
> 
> Either remove all that stuff or fill in the details?

Okay. Will remove this.

>> WebCore/platform/gtk/RenderThemeGtk.cpp:456
>> +                   gtkTextDirection(renderObject->style()->direction()),
> 
> This is somewhat random :D (I believe in general adding extra newlines goes against the style guidelines?)

Oh right. This is unrelated. If a line is longer than 100 or 120 characters it's generally okay to break it though. I will remove this change though.

>> WebCore/platform/gtk/RenderThemeGtk3.cpp:156
>> +#endif
> 
> This is always going to be FALSE for GTK+ 3.x right?

Yep. I'll remove this.
Comment 6 Martin Robinson 2010-12-30 16:10:30 PST
Created attachment 77699 [details]
Patch with Xan's suggestions
Comment 7 WebKit Review Bot 2010-12-30 16:13:32 PST
Attachment 77699 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'WebCore/ChangeLog', u'WebCore/GNUmakefile.am', u'WebCore/platform/gtk/RenderThemeGtk.cpp', u'WebCore/platform/gtk/RenderThemeGtk.h', u'WebCore/platform/gtk/RenderThemeGtk2.cpp', u'WebCore/platform/gtk/RenderThemeGtk3.cpp']" exit_code: 1
WebCore/platform/gtk/RenderThemeGtk2.cpp:164:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/RenderThemeGtk2.cpp:340:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/RenderThemeGtk3.cpp:160:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/RenderThemeGtk3.cpp:333:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 4 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Xan Lopez 2010-12-30 16:45:44 PST
Comment on attachment 77699 [details]
Patch with Xan's suggestions

r=me
Comment 9 Martin Robinson 2010-12-30 17:04:29 PST
Committed r74817: <http://trac.webkit.org/changeset/74817>