Bug 84009

Summary: Introduce an internal feature for a fixed placeholder
Product: WebKit Reporter: Kent Tamura <tkent>
Component: FormsAssignee: Kent Tamura <tkent>
Status: RESOLVED FIXED    
Severity: Normal CC: haraken, morrita, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 83872    
Attachments:
Description Flags
Patch
none
Patch 2
none
Patch 3
none
Patch 4 none

Kent Tamura
Reported 2012-04-15 23:29:49 PDT
Introduce an internal feature for a fixed placeholder
Attachments
Patch (6.37 KB, patch)
2012-04-15 23:38 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Patch 2 (6.46 KB, patch)
2012-04-16 01:38 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Patch 3 (6.40 KB, patch)
2012-04-16 01:39 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Patch 4 (6.65 KB, patch)
2012-04-17 00:09 PDT, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2012-04-15 23:38:52 PDT
Created attachment 137282 [details] Patch
Kentaro Hara
Comment 2 2012-04-16 01:22:36 PDT
Comment on attachment 137282 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137282&action=review > Source/WebCore/html/InputType.h:243 > + // If supportsPlaceholder() && defaultPlaceholder().isEmpty(), it means a > + // type supports the 'placeholder' attribute. > + // If supportsPlaceholder() && !defaultPlaceholder().isEmpty(), it means a > + // type doesn't support the 'placeholder' attribute, but shows > + // defaultPlaceholder() string as a placeholder. Nit: It might be better to put this comment above supportsPlaceholder(). > Source/WebCore/html/TextFieldInputType.cpp:397 > + if (placeholderText.isEmpty()) Shouldn't this be 'if (!placeholderText.isEmpty())'?
Kent Tamura
Comment 3 2012-04-16 01:37:15 PDT
Comment on attachment 137282 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137282&action=review >> Source/WebCore/html/InputType.h:243 >> + // defaultPlaceholder() string as a placeholder. > > Nit: It might be better to put this comment above supportsPlaceholder(). It's reasonable. I'll do. >> Source/WebCore/html/TextFieldInputType.cpp:397 >> + if (placeholderText.isEmpty()) > > Shouldn't this be 'if (!placeholderText.isEmpty())'? No. If defaultPlaceholder() is empty, we'll use the 'placeholder' attribute value.
Kent Tamura
Comment 4 2012-04-16 01:38:27 PDT
Created attachment 137302 [details] Patch 2 Fix a comment
Kent Tamura
Comment 5 2012-04-16 01:39:55 PDT
Created attachment 137303 [details] Patch 3
Kentaro Hara
Comment 6 2012-04-16 01:40:41 PDT
Comment on attachment 137282 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137282&action=review >>> Source/WebCore/html/TextFieldInputType.cpp:397 >>> + if (placeholderText.isEmpty()) >> >> Shouldn't this be 'if (!placeholderText.isEmpty())'? > > No. If defaultPlaceholder() is empty, we'll use the 'placeholder' attribute value. Ah, got it. I'd been confused. > Source/WebCore/html/TextFieldInputType.cpp:398 > + placeholderText = element()->strippedPlaceholder(); Nit: You can move this statement inside the below if statement.
Kentaro Hara
Comment 7 2012-04-16 01:41:41 PDT
(In reply to comment #6) > > Source/WebCore/html/TextFieldInputType.cpp:398 > > + placeholderText = element()->strippedPlaceholder(); > > Nit: You can move this statement inside the below if statement. Sorry, ignore this comment. You code is right.
Kent Tamura
Comment 8 2012-04-17 00:09:47 PDT
Created attachment 137486 [details] Patch 4 Add InputType::usesFixedPlaceholder()
WebKit Review Bot
Comment 9 2012-04-17 01:36:57 PDT
Comment on attachment 137486 [details] Patch 4 Clearing flags on attachment: 137486 Committed r114360: <http://trac.webkit.org/changeset/114360>
WebKit Review Bot
Comment 10 2012-04-17 01:37:02 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.