Bug 30223 - [LayoutTests][Gtk] Set a common Gtk theme as default and update the results
Summary: [LayoutTests][Gtk] Set a common Gtk theme as default and update the results
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-08 10:54 PDT by Zan Dobersek
Modified: 2009-10-15 13:37 PDT (History)
3 users (show)

See Also:


Attachments
Set Raleigh as the default theme (1.32 KB, patch)
2009-10-08 11:13 PDT, Zan Dobersek
eric: review+
eric: commit-queue-
Details | Formatted Diff | Diff
Set Raleigh as the default theme (1.19 KB, patch)
2009-10-09 00:49 PDT, Zan Dobersek
zecke: review+
Details | Formatted Diff | Diff
Updated results (4.48 KB, patch)
2009-10-11 10:46 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2009-10-08 10:54:28 PDT
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.
Comment 1 Zan Dobersek 2009-10-08 11:13:59 PDT
Created attachment 40894 [details]
Set Raleigh as the default theme

Sets Raleigh as the default theme.
Comment 2 Eric Seidel (no email) 2009-10-08 12:06:05 PDT
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.
Comment 3 Evan Martin 2009-10-08 12:22:51 PDT
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?)
Comment 4 Zan Dobersek 2009-10-09 00:35:50 PDT
(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.
Comment 5 Zan Dobersek 2009-10-09 00:49:37 PDT
Created attachment 40935 [details]
Set Raleigh as the default theme

Replaces use of wrapped lines.
Comment 6 Holger Freyther 2009-10-10 05:46:43 PDT
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?
Comment 7 Zan Dobersek 2009-10-11 10:46:16 PDT
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.
Comment 8 Holger Freyther 2009-10-13 18:41:28 PDT
Comment on attachment 41001 [details]
Updated results

sure.
Comment 9 WebKit Commit Bot 2009-10-13 20:37:33 PDT
Comment on attachment 41001 [details]
Updated results

Clearing flags on attachment: 41001

Committed r49552: <http://trac.webkit.org/changeset/49552>
Comment 10 WebKit Commit Bot 2009-10-13 20:37:37 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Eric Seidel (no email) 2009-10-15 13:37:15 PDT
Comment on attachment 40935 [details]
Set Raleigh as the default theme

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