Bug 33093 – [Qt] LayoutTests/fast/html/text-field-input-types.html

• Component: WebKit Qt
• Status: RESOLVED FIXED

[Robert Hogan]

This test is skipped on Qt because it's output is:

A B C DE F

rather than

A B C D E F.

In the dumped text the spaces represent an <input> text field.

The reason for the difference is that on Mac all the input fields fit on a single line, whereas on Qt 'D' starts on a new line. This happens because the width() of each of the <input> controls on Qt is 175, which must be larger than it is on Mac. The 175 is arrived at by taking the avgCharWidth of the render style's font (8) and multiplying it by InputElement::s_defaultSize (20), then adding the padding/margins. So the difference must stem from a different avgCharWidth between Mac and Qt.

What's the right thing to do here? Is it a bug that the text dump does not begin on a new line with E and F?

The test itself can be fixed for all platforms by specifying a small size attribute in the <input> markup, maybe that's the right way to go.

[Chang Shu]

Created attachment 48344 [details]
fix patch

[Adam Barth]

Comment on attachment 48344 [details]
fix patch

Hum...  I think this patch is correct.  There's a separate question of whether you want the Qt metrics to match Mac, but that's not what this test is trying to test, so I think that's better dealt with separately.

I agree that it's possible we should improve the text dumping to be immune to these issues.  If this issue recurs, we can revisit that question.

BTW, it would be helpful if you'd copy more of your analysis from Comment #0 into the ChangeLog.  I found Comment #0 quite informative but you ChangeLog somewhat mysterious.

[Chang Shu]

(In reply to comment #2)
> (From update of attachment 48344 [details])
> Hum...  I think this patch is correct.  There's a separate question of whether
> you want the Qt metrics to match Mac, but that's not what this test is trying
> to test, so I think that's better dealt with separately.
> I agree that it's possible we should improve the text dumping to be immune to
> these issues.  If this issue recurs, we can revisit that question.
> BTW, it would be helpful if you'd copy more of your analysis from Comment #0
> into the ChangeLog.  I found Comment #0 quite informative but you ChangeLog
> somewhat mysterious.

Thanks for the review, Adam. And thanks to Robert, too. It is actually Robert who did all the analysis. I made this patch simply because this bug blocked my further work on making some of the input types working on Qt. And I also think the intention of the test case is somewhere else. Yes, I will add Robert's comment into the ChangLog.

[Chang Shu]

Created attachment 48701 [details]
fix patch 2

[Adam Barth]

Comment on attachment 48701 [details]
fix patch 2

Thanks.

[WebKit Commit Bot]

Comment on attachment 48701 [details]
fix patch 2

Clearing flags on attachment: 48701

Committed r54761: <http://trac.webkit.org/changeset/54761>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.