[Webkit-unassigned] [Bug 21908] size attribute has no effect for input tag file type

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=21908


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #26538|review?                     |review-
               Flag|                            |




------- Comment #9 from darin at apple.com  2009-01-10 14:51 PDT -------
(From update of attachment 26538)
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 @@
> +2009-01-07  Alpha Lam  <hclam at google.com>
> +
> +        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.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.