Allow platforms to specify if the placeholder should be visible when text controls are focused
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