Bug 70474 - [Forms][File] Add tooltip to "No file selected" text
Summary: [Forms][File] Add tooltip to "No file selected" text
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-19 22:21 PDT by yosin
Modified: 2011-10-20 19:15 PDT (History)
6 users (show)

See Also:


Attachments
Patch 1 (11.25 KB, patch)
2011-10-20 00:23 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 2 (11.44 KB, patch)
2011-10-20 00:41 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch (6.51 KB, patch)
2011-10-20 01:34 PDT, yosin
no flags Details | Formatted Diff | Diff
Patch 4 (6.92 KB, patch)
2011-10-20 02:06 PDT, yosin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2011-10-19 22:21:13 PDT
When width of file input element isn't enough for displaying "No file selected" text is truncated. There is no way for users to see entire text.

Sample HTML:
<input type="file" style="border: solid 1px red; width: 150px">

Sample Rendering:
[Choose File] No f....sen

This change fixes following Chromium bug:
Message 'no file selected' is not completed in Czech and some other languages
http://crbug.com/23354
Comment 1 yosin 2011-10-20 00:23:59 PDT
Created attachment 111733 [details]
Patch 1
Comment 2 WebKit Review Bot 2011-10-20 00:28:37 PDT
Comment on attachment 111733 [details]
Patch 1

Attachment 111733 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10179350
Comment 3 Gyuyoung Kim 2011-10-20 00:30:49 PDT
Comment on attachment 111733 [details]
Patch 1

Attachment 111733 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/10178336
Comment 4 Early Warning System Bot 2011-10-20 00:31:33 PDT
Comment on attachment 111733 [details]
Patch 1

Attachment 111733 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/10180386
Comment 5 yosin 2011-10-20 00:41:34 PDT
Created attachment 111735 [details]
Patch 2
Comment 6 Kent Tamura 2011-10-20 00:47:39 PDT
Comment on attachment 111735 [details]
Patch 2

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

> Source/WebCore/ChangeLog:5
> +        Changes for check-webkit-style

Do not include style changes.  They are unrelated to the bug.

Also, please write the reason why you change the behavior.

> Source/WebCore/ChangeLog:22
> +        * html/FileInputType.cpp:
> +        (WebCore::FileInputType::getToolTip):
> +        * html/FileInputType.h:
> +        * html/HTMLInputElement.cpp:
> +        (WebCore::HTMLInputElement::parseMappedAttribute):
> +        (WebCore::HTMLInputElement::getToolTip):
> +        * html/HTMLInputElement.h:
> +        * html/InputType.cpp:
> +        (WebCore::InputType::getToolTip):
> +        * html/InputType.h:
> +        * page/Chrome.cpp:
> +        (WebCore::Chrome::setToolTip):

Please write what you changed for each of files/functions as possible.

> Source/WebCore/html/FileInputType.h:68
> +    virtual String getToolTip() const;

Please append OVERRIDE.

> Source/WebCore/html/HTMLInputElement.cpp:54
> -#include "SearchInputType.h"
>  #include "ScriptEventListener.h"
> +#include "SearchInputType.h"

Do not change this in this patch.

> Source/WebCore/html/HTMLInputElement.cpp:800
> -        // FIXME: Detaching just for maxResults change is not ideal.  We should figure out the right
> +        // FIXME: Detaching just for maxResults change is not ideal. We should figure out the right

ditto.

> Source/WebCore/html/HTMLInputElement.h:59
> -    // Returns the minimum value for type=date, number, or range.  Don't call this for other types.
> +    // Returns the minimum value for type=date, number, or range. Don't call this for other types.
>      double minimum() const;
> -    // Returns the maximum value for type=date, number, or range.  Don't call this for other types.
> +    // Returns the maximum value for type=date, number, or range. Don't call this for other types.

ditto.

> Source/WebCore/html/HTMLInputElement.h:320
> -    
> +

ditto.

> Source/WebCore/html/HTMLInputElement.h:325
> -    // Helper for stepUp()/stepDown().  Adds step value * count to the current value.
> +    // Helper for stepUp()/stepDown(). Adds step value * count to the current value.

ditto.

> Source/WebCore/html/HTMLInputElement.h:357
> -} //namespace
> +} // namespace

ditto.

> Source/WebCore/html/InputType.h:36
> -#include <wtf/Forward.h>
>  #include <wtf/FastAllocBase.h>
> +#include <wtf/Forward.h>

ditto.

> Source/WebCore/html/InputType.h:77
> -    WTF_MAKE_NONCOPYABLE(InputType); WTF_MAKE_FAST_ALLOCATED;
> +    WTF_MAKE_NONCOPYABLE(InputType);
> +    WTF_MAKE_FAST_ALLOCATED;

ditto.

> Source/WebCore/page/Chrome.cpp:28
> -#include "FileIconLoader.h"
>  #include "FileChooser.h"
> +#include "FileIconLoader.h"

ditto.

> Source/WebCore/page/Chrome.cpp:424
> +                // TODO(yosin): We should obtain text direction of tooltip

TODO(name) is chromium-style. We should change it to FIXME: .
Comment 7 yosin 2011-10-20 01:34:42 PDT
Created attachment 111739 [details]
Patch
Comment 8 Kent Tamura 2011-10-20 01:40:50 PDT
Comment on attachment 111739 [details]
Patch

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

> Source/WebCore/ChangeLog:25
> +        * html/FileInputType.cpp:
> +        (WebCore::FileInputType::getToolTip):
> +        * html/FileInputType.h:
> +        * html/HTMLInputElement.cpp:
> +        (WebCore::HTMLInputElement::getToolTip):
> +        * html/HTMLInputElement.h:
> +        * html/InputType.cpp:
> +        (WebCore::InputType::getToolTip):
> +        * html/InputType.h:
> +        * page/Chrome.cpp:
> +        (WebCore::Chrome::setToolTip):

Please write what is changed for each of files/functions as possible.

> Source/WebCore/html/HTMLInputElement.h:236
> +    String getToolTip() const;
> +

Please see item 6 and 7 of 'Names' section of http://www.webkit.org/coding/coding-style.html.
This function should be toolTip()", not getToolTip().

Also, I think defaultToolTip() or fallbackToolTip() is better.

> Source/WebCore/html/InputType.h:241
> +    virtual String getToolTip() const;

ditto.
Comment 9 yosin 2011-10-20 02:06:31 PDT
Created attachment 111743 [details]
Patch 4
Comment 10 Kent Tamura 2011-10-20 18:31:57 PDT
Comment on attachment 111743 [details]
Patch 4

Looks good.
Comment 11 WebKit Review Bot 2011-10-20 19:08:00 PDT
Comment on attachment 111743 [details]
Patch 4

Clearing flags on attachment: 111743

Committed r98054: <http://trac.webkit.org/changeset/98054>
Comment 12 WebKit Review Bot 2011-10-20 19:08:05 PDT
All reviewed patches have been landed.  Closing bug.