Bug 95542 - [Forms] AM/PM field of multiple fields time input UI should be fixed width
Summary: [Forms] AM/PM field of multiple fields time input UI should be fixed width
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: yosin
URL:
Keywords:
Depends on: 95545
Blocks: 95660
  Show dependency tree / graph
 
Reported: 2012-08-31 02:42 PDT by yosin
Modified: 2012-09-02 21:02 PDT (History)
5 users (show)

See Also:

Attachments
Patch 1 (5.35 KB, patch)
2012-08-31 03:57 PDT, yosin 		no flags Details | Formatted Diff | Diff
Patch 2 (5.15 KB, patch)
2012-09-02 18:34 PDT, yosin 		no flags Details | Formatted Diff | Diff
Patch 3 (5.12 KB, patch)
2012-09-02 19:06 PDT, yosin 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description yosin 2012-08-31 02:42:06 PDT 
For Japanese, AM/PM symbols are in Kanji, these texts are longer than "--".
For Arabic, AM/PM symbols are one character, these characters are shorter than "--".
Comment 1 yosin 2012-08-31 03:57:02 PDT 
Created attachment 161656 [details]
Patch 1
Comment 2 yosin 2012-09-02 18:34:58 PDT 
Created attachment 161852 [details]
Patch 2
Comment 3 yosin 2012-09-02 18:35:40 PDT 
Comment on attachment 161852 [details]
Patch 2

Could you review this patch?
Thanks in advance.
Comment 4 Kent Tamura 2012-09-02 18:47:57 PDT 
Comment on attachment 161852 [details]
Patch 2

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

> Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:35
> +#include "TextRun.h"
> +#include <wtf/text/StringBuilder.h>

Are TextRun.h and StringBuilder.h needed?
Comment 5 yosin 2012-09-02 19:05:56 PDT 
(In reply to comment #4)
> (From update of attachment 161852 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=161852&action=review
> 
> > Source/WebCore/html/shadow/DateTimeSymbolicFieldElement.cpp:35
> > +#include "TextRun.h"
> > +#include <wtf/text/StringBuilder.h>
> 
> Are TextRun.h and StringBuilder.h needed?
TextRun.h is needed for style()->font().width().
StringBuilder.h isn't needed.
Comment 6 yosin 2012-09-02 19:06:09 PDT 
Created attachment 161854 [details]
Patch 3
Comment 7 yosin 2012-09-02 19:07:25 PDT 
Comment on attachment 161854 [details]
Patch 3

Could you review this patch?
Thanks in advance.

= Changes since the last review =
* Remove extra include file StringBuilder.h from DateTimeSymbolicFieldElement.cpp
Comment 8 Kent Tamura 2012-09-02 20:04:21 PDT 
Comment on attachment 161854 [details]
Patch 3

ok
Comment 9 yosin 2012-09-02 21:02:52 PDT 
Comment on attachment 161854 [details]
Patch 3

Clearing flags on attachment: 161854

Committed r127398: <http://trac.webkit.org/changeset/127398>
Comment 10 yosin 2012-09-02 21:02:57 PDT 
All reviewed patches have been landed.  Closing bug.