Summary: | Allow platforms to specify if the placeholder should be visible when text controls are focused | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adele Peterson <adele> | ||||||
Component: | Forms | Assignee: | Adele Peterson <adele> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | adele, darin, dglazkov, myakura.web, tkent | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 63927 | ||||||||
Attachments: |
|
Description
Adele Peterson
2011-02-03 17:07:36 PST
Created attachment 81151 [details]
Patch
Comment on attachment 81151 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81151&action=review > Source/WebCore/html/HTMLFormControlElement.cpp:586 > return supportsPlaceholder() > && isEmptyValue() > - && document()->focusedNode() != this > - && !isPlaceholderEmpty(); > + && !isPlaceholderEmpty() > + && document()->focusedNode() != this || (renderer() && renderer()->theme()->shouldShowPlaceholderWhenFocused()); I was really confused by this but finally determined that you meant for this last line to be one term in the chain of && conditions. However, right now it isn’t. I think you need parentheses around the last line. Doh! I thought I had worked it out right, and at the last minute removed those parenthesis. I will re-add them… (In reply to comment #2) > (From update of attachment 81151 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=81151&action=review > > > Source/WebCore/html/HTMLFormControlElement.cpp:586 > > return supportsPlaceholder() > > && isEmptyValue() > > - && document()->focusedNode() != this > > - && !isPlaceholderEmpty(); > > + && !isPlaceholderEmpty() > > + && document()->focusedNode() != this || (renderer() && renderer()->theme()->shouldShowPlaceholderWhenFocused()); > > I was really confused by this but finally determined that you meant for this last line to be one term in the chain of && conditions. However, right now it isn’t. I think you need parentheses around the last line. Created attachment 81161 [details]
Patch
Comment on attachment 81161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81161&action=review You are missing a LayoutTests/ChangeLog. > Source/WebCore/html/HTMLTextAreaElement.cpp:280 > + const_cast<HTMLTextAreaElement*>(this)->updatePlaceholderVisibility(false); Is this not needed for text fields too? Comment on attachment 81161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81161&action=review Thanks! >> Source/WebCore/html/HTMLTextAreaElement.cpp:280 >> + const_cast<HTMLTextAreaElement*>(this)->updatePlaceholderVisibility(false); > > Is this not needed for text fields too? Text fields update the value at a slightly different time, and the placeholder already gets updated at that time. Committed revision 77639. Maybe we should do this too for Chromium. Kent-san, WDYT? So, will Lion show placeholders for focused text fields? I have received some requests for this new behavior. But the HTML5 clearly specifies the current behavior. [1] Should we ask to update the HTML5 specification so that the behavior depends on platforms? [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#the-placeholder-attribute (In reply to comment #9) > So, will Lion show placeholders for focused text fields? LIon removes placeholder when the element got an input, not on focus. > I have received some requests for this new behavior. But the HTML5 clearly specifies the current behavior. [1] > Should we ask to update the HTML5 specification so that the behavior depends on platforms? > > > > [1] http://www.whatwg.org/specs/web-apps/current-work/multipage/common-input-element-attributes.html#the-placeholder-attribute I filed a bug asking for that and the spec changed today. So now you can answer to the requests without breaking spec conformance :-) http://html5.org/r/6782 |