size attribute has no effect for input tag file type

• Component: Layout and Rendering
• Status: NEW ---

[Hin-Chung Lam]

### PROBLEM ###
Although the input elements have different size attribute values, they 
have the same width.

### EXPECTED ###
The size attribute takes effect.

Chrome 0.3.154.0
Windows XP SP2 en

Not repro on Firefox 3.0.3, IE7 and IE8 beta2.
Repro on Safari 3.1.2.

Originally found in Chrome, please refer to the following link for samples:
http://code.google.com/p/chromium/issues/detail?id=3238

[Hin-Chung Lam]

Created attachment 24699 [details]
bug sample

[Hin-Chung Lam]

Created attachment 24700 [details]
bug image

[Hin-Chung Lam]

Created attachment 24727 [details]
patch for r37934

[Hin-Chung Lam]

Created attachment 26520 [details]
patch for code, added layout test and modify old layout tests

[Hin-Chung Lam]

The bug happens in nightly build too

[Darin Adler]

Comment on attachment 26520 [details]
patch for code, added layout test and modify old layout tests

I think the current behavior is intentional. As a security measure, browsers don't allow styling of <input type="file">. Perhaps this long-standing security measure is obsolete, though. We'd have to decide this before making a change.

Also, this ChangeLog entries in this patch have tab characters in them. We don't allow those.

[Hin-Chung Lam]

Created attachment 26538 [details]
patch the code and updated layout tests

[Hin-Chung Lam]

removed tabs in ChangeLogs

[Darin Adler]

Comment on attachment 26538 [details]
patch the code and updated layout tests

OK, I looked more carefully at your comments and it seems you did check the behavior of other browsers.

> -        m_maxPrefWidth = (int)ceilf(charWidth * defaultWidthNumChars);
> +        m_maxPrefWidth = (int)ceilf(charWidth * static_cast<HTMLInputElement*>(node())->size()
> +                         + m_button->renderer()->maxPrefWidth() + afterButtonSpacing
> +                         + (m_fileChooser->icon() ? iconWidth + iconFilenameSpacing : 0));

Will this give us the correct behavior if someone sets the size attribute to zero or a large negative number? Do we need some kind of bounds check on the value of size() or can we just use the value no matter what integer it is? What do other browsers do in these edge cases? It also seems clear that this could overflow if the value of size is something gigantic.

The old default size was 34 characters, but the default value for HTMLInputElement's size is 20. We do add space for the other elements. But does this make the default size smaller or larger?

Is it really sensible to treat the size attribute as "number of characters of average width available for the filename"? It seems a little strange to me, and I'm not sure that's the most helpful behavior for website authors.

The defaultWidthNumChars constant is now unused. Please remove it if you're removing the last use of it.

> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog	(revision 39698)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,35 @@
> [phone]  Alpha Lam  <[e-mail]>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +       Added layout test for size property of <input type="file"> tag, updating other layout tests related to issue.
> +
> +       https://bugs.webkit.org/show_bug.cgi?id=21908

These are indented incorrectly. 7 spaces rather than 8.

> Index: LayoutTests/fast/replaced/table-percent-height-expected.txt
> ===================================================================
> --- LayoutTests/fast/replaced/table-percent-height-expected.txt	(revision 39697)
> +++ LayoutTests/fast/replaced/table-percent-height-expected.txt	(working copy)
> @@ -63,9 +63,9 @@ PASS getComputedStyleForElement(document
>  PASS getComputedStyleForElement(document.getElementById('input-checkbox-75'), 'height') is '4px'
>  PASS getComputedStyleForElement(document.getElementById('input-checkbox-100'), 'width') is '12px'
>  PASS getComputedStyleForElement(document.getElementById('input-checkbox-100'), 'height') is '6px'
> -PASS getComputedStyleForElement(document.getElementById('input-file-75'), 'width') is '237px'
> +FAIL getComputedStyleForElement(document.getElementById('input-file-75'), 'width') should be 237px. Was 222px.
>  PASS getComputedStyleForElement(document.getElementById('input-file-75'), 'height') is '13px'
> -PASS getComputedStyleForElement(document.getElementById('input-file-100'), 'width') is '237px'
> +FAIL getComputedStyleForElement(document.getElementById('input-file-100'), 'width') should be 237px. Was 222px.

I think we should do better than just checking in failing test results here. If the results should change, then we should change the expected results in the test too. If the results should not change, then lets not make the code change.

I'm going to say review- because I think that the non-trivial concerns above should be resolved before making this change.

[Ahmad Saleem]

I am not able to reproduce the issue of reference bug but this is different in Safari compared to others:

Tested on WebKit ToT (263769@main):

In attached testcase for 'style width', for some cases, we don't shrink 'button' to accommodate text while we overlap text above input button while Firefox Nightly 114 and Chrome Canary shrink button or cut-off it (e.g., look at 20px: and 40px: etc. in all browsers).