Bug 51755 – [GTK] Fork pieces of RenderThemeGtk that will differ for GTK+ 3

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

[Martin Robinson]

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.

[Martin Robinson]

Created attachment 77687 [details]
Split out methods that will differ

[WebKit Review Bot]

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.

[Martin Robinson]

(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

[Xan Lopez]

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?

[Martin Robinson]

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.

[Martin Robinson]

Created attachment 77699 [details]
Patch with Xan's suggestions

[WebKit Review Bot]

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.

[Xan Lopez]

Comment on attachment 77699 [details]
Patch with Xan's suggestions

r=me

[Martin Robinson]

Committed r74817: <http://trac.webkit.org/changeset/74817>