Bug 30223 – [LayoutTests][Gtk] Set a common Gtk theme as default and update the results

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Zan Dobersek]

At the moment, unexpected failures can occur because of running the tests while having different Gtk themes set as default. This appears on the Gtk buildbots, where tests are run under Xvfb, in which Raleigh is used as the default theme. Running the tests with a different theme can produce failures.

The Raleigh theme is also Gtk's default and is as such present on every Gtk system. Therefor, I recommend setting this theme as default in the DumpRenderTree.

This bug should be considered for closing when a default theme is set and the results are updated to reflect the changes.

[Zan Dobersek]

Created attachment 40894 [details]
Set Raleigh as the default theme

Sets Raleigh as the default theme.

[Eric Seidel (no email)]

Comment on attachment 40894 [details]
Set Raleigh as the default theme

Sounds fine to me.  Seems silly to wrap this line when lines just below it have no wrapping.  WebKit style has no official column width.

Also WebKit style is to use 0 instead of NULL for c++ code, although maybe the gtk code follows some other style?  check-webkit-style should help identify some style issues.

looks fine, but cq- for the style oddities.

[Evan Martin]

Maybe consider setting the font names and sizes as well?  But maybe that's a different bug.  If you're really loading all of this from GtkSettings you'll want to override all of 'em, including cursor-blink etc.

(Is this committable without test rebaselines?  Or are they all baselined to Raleigh?)

[Zan Dobersek]

(In reply to comment #2)
> (From update of attachment 40894 [details])
> 
> Also WebKit style is to use 0 instead of NULL for c++ code, although maybe the
> gtk code follows some other style?  check-webkit-style should help identify
> some style issues.
> 

I'll raise a discussion on the irc channel about 0 vs NULL problem. I'll also see about opening a bug for style cleanup of Gtk port's code.

(In reply to comment #3)
> Maybe consider setting the font names and sizes as well?  But maybe that's a
> different bug.  If you're really loading all of this from GtkSettings you'll
> want to override all of 'em, including cursor-blink etc.

Font families and sizes are taken care of in resetDefaultsToConsistentValues function, which is called before each test. I don't think overriding any other setting would do any good, or harm.

If it, however, appears that it should be done, the code should be updated - moving all of the overrides of GtkSettings into a new function for a better overview.

> (Is this committable without test rebaselines?  Or are they all baselined to
> Raleigh?)

Rebaselining patch will be attached to this bug asap.

[Zan Dobersek]

Created attachment 40935 [details]
Set Raleigh as the default theme

Replaces use of wrapped lines.

[Holger Freyther]

Comment on attachment 40935 [details]
Set Raleigh as the default theme

Using NULL as sentinel is what we seem to do now and I see little reason to change that. Could you please watch the bot when landing it?

[Zan Dobersek]

Created attachment 41001 [details]
Updated results

This updates results for two tests that were failing on the buildbots because results were generated in a theme different than Raleigh.

[Holger Freyther]

Comment on attachment 41001 [details]
Updated results

sure.

[WebKit Commit Bot]

Comment on attachment 41001 [details]
Updated results

Clearing flags on attachment: 41001

Committed r49552: <http://trac.webkit.org/changeset/49552>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Comment on attachment 40935 [details]
Set Raleigh as the default theme

Removing cq? on this closed bug.  I assume this patch was already landed.