Bug 80712 - [CSS Regions] Convert some fast/regions pixel tests to reftests
Summary: [CSS Regions] Convert some fast/regions pixel tests to reftests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 63344
  Show dependency treegraph
 
Reported: 2012-03-09 11:07 PST by David Alcala
Modified: 2013-04-30 09:56 PDT (History)
9 users (show)

See Also:


Attachments
Patch (36.92 KB, patch)
2013-04-29 07:05 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch (308.08 KB, patch)
2013-04-29 07:11 PDT, Radu Stavila
achicu: review-
Details | Formatted Diff | Diff
Patch (308.51 KB, patch)
2013-04-30 03:49 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Alcala 2012-03-09 11:07:46 PST
Convert the fast/regions/webkit-flow* pixel tests to reftests
Comment 1 Radu Stavila 2013-04-29 07:05:30 PDT
Created attachment 200003 [details]
Patch
Comment 2 Radu Stavila 2013-04-29 07:11:49 PDT
Created attachment 200005 [details]
Patch
Comment 3 Alexandru Chiculita 2013-04-29 09:24:08 PDT
Comment on attachment 200005 [details]
Patch

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

Thanks, looks good! I have a couple of hints below. The comments apply for both html files.

> LayoutTests/fast/regions/webkit-flow-inlines-inside-regions-bounds-expected.html:5
> +    	text-align: justify;

nit: you have different indentation styles in the style elements.

> LayoutTests/fast/regions/webkit-flow-inlines-inside-regions-bounds-expected.html:87
> +        margin-top: -22px;

<p> tags have margin-top/bottom set using "em" units.  I think you could avoid using <p> tags or set the margins to 0 to avoid having to compensate for it in the parent box. Setting the margin this way makes the ref-test fragile when changing the default font-size.
Comment 4 Radu Stavila 2013-04-30 03:49:12 PDT
Created attachment 200097 [details]
Patch

Made recommended changes
Comment 5 Radu Stavila 2013-04-30 08:26:26 PDT
Remaining tests will be tracked in https://bugs.webkit.org/show_bug.cgi?id=115422
Comment 6 Alexandru Chiculita 2013-04-30 09:26:34 PDT
Comment on attachment 200097 [details]
Patch

Looks better.
R=me

You need to set the CQ? flag if you want the reviewer to set the CQ+ flag.
Comment 7 WebKit Commit Bot 2013-04-30 09:56:31 PDT
Comment on attachment 200097 [details]
Patch

Clearing flags on attachment: 200097

Committed r149374: <http://trac.webkit.org/changeset/149374>
Comment 8 WebKit Commit Bot 2013-04-30 09:56:35 PDT
All reviewed patches have been landed.  Closing bug.