Bug 29502 – basic inputs layout test for chromium

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Karen]

This tests basic inputs for chromium to make sure they render properly.

[Karen]

Created attachment 39784 [details]
basic inputs layout test patch

[Pam Greene (IRC:pamg)]

A test for any WebKit port, really. It happens to have been written for Chromium, but any new port might find it useful.

[Karen]

Created attachment 39787 [details]
basic inputs layout test patch - fixed patch.

[Eric Seidel (no email)]

Comment on attachment 39787 [details]
basic inputs layout test patch - fixed patch.

The tab in the ChangeLog will make this unlandable.  (We have a pre-commit hook which rejects commits with tabs in them.)

The ChangeLog shoudl also explain more what you're doing here.  Why this test, and what does it test that other tests don't?

[Karen]

Created attachment 39790 [details]
basic inputs layout test patch - added more info to changelog.

i added more info to the changelog. this patch is for a layout test to thoroughly test input fields. text and password, enabled and disabled.

[Eric Seidel (no email)]

Comment on attachment 39790 [details]
basic inputs layout test patch - added more info to changelog.

This also has tabs in the ChangeLog, which will make it impossible to land automatically. :(

Missing a couple capital letters at the start of your sentences.

prepare-ChangeLog (the ChangeLog template generation tool) takes a --bug argument which automatically inserts the bug url in the expected format in case you need it in the future.  Generally the URL goes on its own line.

So asside from the ChagneLog nits, this looks fine.  but r- for the tabs.

[Pam Greene (IRC:pamg)]

Any test containing an input field implicitly depends on these features, but it's difficult for a new port to isolate differences. It is helpful to have some tests that are intended only to document and verify the sizes and behavior of basic input fields.

[Karen]

Created attachment 39794 [details]
basic inputs layout test patch - fixed tabs in changelog, capitalized.

[Karen]

Created attachment 39797 [details]
fixed tabs in changelog.

[Eric Seidel (no email)]

Comment on attachment 39797 [details]
fixed tabs in changelog.

Now you have an extra space at the top of your ChangeLog file. :(

You might find http://webkit.org/coding/contributing.html#changelogs of interest if you haven't already seen it.

Otherwise this looks fine.  I would r+ this, except you're not a committer so it has to be perfect for the commit bot to land this for you. :(

While we're at it, "For chomium" is not a sentence, I assume you added an extra period by mistake:
This tests all types of inputs text and password, both enabled and disabled, the existing tests weren't thorough enough. For chromium.

[Karen]

Created attachment 39800 [details]
basic inputs layout test patch - fixed space in changelog and removed period.

[Dimitri Glazkov (Google)]

Comment on attachment 39800 [details]
basic inputs layout test patch - fixed space in changelog and removed period.

> +  border:1px solid red;
> +  margin:10px;

4-space indent.

> +<div>
> +  a<input value="foobarbazfoobarbazfoobarbaz" type="text" />text
> +  <input value="foo" type="text" DISABLED />b
> +  a<input value="foo" type="password" />password
> +  <input value="foo" type="password" DISABLED />b
> +</div>

ditto here and on.

[Karen]

Created attachment 40028 [details]
basic inputs layout test patch - fixed indents and changelog

basic inputs layout test patch - fixed indents and changelog

[Eric Seidel (no email)]

Comment on attachment 40028 [details]
basic inputs layout test patch - fixed indents and changelog

LGTM.  I'm assuming you wanted this commit-queued since this won't break the canary (except in missing test results, which theoretically shouldn't be missing anyway).

[WebKit Commit Bot]

Comment on attachment 40028 [details]
basic inputs layout test patch - fixed indents and changelog

Clearing flags on attachment: 40028

Committed r48699: <http://trac.webkit.org/changeset/48699>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.