Implement text field placeholders using shadow DOM

• Component: Forms
• Status: RESOLVED FIXED

[Kent Tamura]

In order to fix multiple placeholder regressions, adds new shadow elements (and text nodes) representing placeholders.

[Kent Tamura]

Created attachment 100258 [details]
WIP

[Kent Tamura]

(In reply to comment #1)
> Created an attachment (id=100258) [details]
> WIP

The patch still has a problem:
If a placeholder box for a textarea is higher than the textarea, a horizontal scrollbar is shown.

[Dimitri Glazkov (Google)]

I tweaked the title a bit, sorry :)

[Dimitri Glazkov (Google)]

Comment on attachment 100258 [details]
WIP

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

Looking forward to the final patch!

> Source/WebCore/rendering/RenderTextControl.cpp:143
> +    if (RenderStyle* pseudoStyle = getCachedPseudoStyle(INPUT_PLACEHOLDER))
> +        style = RenderStyle::clone(pseudoStyle);
> +    else {
> +        style = RenderStyle::create();
> +        style->inheritFrom(startStyle);
> +    }
> +    style->setTextSecurity(TSNONE);
> +    style->setPosition(RelativePosition);
> +    style->setDisplay(BLOCK);
> +    style->setVisibility(m_placeholderVisible ? VISIBLE : HIDDEN);
> +    style->setPointerEvents(PE_NONE);
> +    return style.release();

Should use shadowPseudoId here.

[Kent Tamura]

Created attachment 100457 [details]
Patch

[WebKit Review Bot]

Attachment 100457 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

ERROR: FAILURES FOR <, x86, release, cpu>
ERROR: Line:6 Path does not exist. fast/form/textarea-placeholder-wrapping.html
LayoutTests/platform/gtk/test_expectations.txt:6:  Path does not exist. fast/form/textarea-placeholder-wrapping.html  [test/expectations] [2]
ERROR: FAILURES FOR <, x86, release, cpu>
ERROR: Line:6 Path does not exist. fast/form/textarea-placeholder-wrapping.html
LayoutTests/platform/qt/test_expectations.txt:6:  Path does not exist. fast/form/textarea-placeholder-wrapping.html  [test/expectations] [2]
ERROR: FAILURES FOR <lucid, x86_64, release, cpu>
ERROR: Line:3788 Path does not exist. fast/form/textarea-placeholder-wrapping.html
LayoutTests/platform/chromium/test_expectations.txt:3788:  Path does not exist. fast/form/textarea-placeholder-wrapping.html  [test/expectations] [2]
Total errors found: 3 in 39 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kent Tamura]

Created attachment 100459 [details]
Patch 2

Fix text_expectations.txt

[WebKit Review Bot]

Comment on attachment 100459 [details]
Patch 2

Attachment 100459 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9016115

New failing tests:
http/tests/misc/slow-loading-mask.html
fast/blockflow/japanese-rl-selection.html
svg/transforms/text-with-mask-with-svg-transform.svg
fast/backgrounds/background-leakage.html
fast/box-shadow/scaled-box-shadow.html
fast/backgrounds/repeat/negative-offset-repeat.html
fast/css/pseudo-cache-stale.html
svg/W3C-SVG-1.1/struct-use-01-t.svg
transforms/2d/hindi-rotated.html
svg/repaint/filter-repaint.svg
fast/blockflow/japanese-lr-selection.html

[WebKit Review Bot]

Created attachment 100460 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Kent Tamura]

Created attachment 100463 [details]
Patch 3

Update for fast/css/pseudo-cache-stale.html

[Dominic Cooney]

Comment on attachment 100463 [details]
Patch 3

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

This is very cool.

The placeholder text in the last textbox in each group in the input-placeholder-text-indent has moved significantly… is that expected?

Why do the pixel results for isindex-placeholder and password-placeholder-text-security need to be updated? Eyeballing them they are the same.

> LayoutTests/fast/forms/placeholder-position.html:16
> +<input style="line-height:25px" value="Value">

For consistency with above maybe:
- put a space after :
- end with a semicolon
- quote the placeholder attribute
- put the placeholder attribute before the inline style

Below too.

[Kent Tamura]

(In reply to comment #11)
> The placeholder text in the last textbox in each group in the input-placeholder-text-indent has moved significantly… is that expected?

It's expected. The placeholders with this patch correctly respect text-indent:50%.

> Why do the pixel results for isindex-placeholder and password-placeholder-text-security need to be updated? Eyeballing them they are the same.

The difference is that the bottom 1px line of a placeholder is not drawn in the updated images. It's because the current placeholder rendering doesn't clip characters by the content-box and the patched placeholder rendering does.

[Dimitri Glazkov (Google)]

Comment on attachment 100463 [details]
Patch 3

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

> Source/WebCore/rendering/RenderTextControlMultiLine.h:52
> +    virtual RenderObject* layoutSpecialExcludedChild(bool relayoutChildren);

Whoa, this is crazy stuff. I didn't realize this mechanism existed.

[Kent Tamura]

Committed r90971: <http://trac.webkit.org/changeset/90971>

[Alexey Proskuryakov]

This has caused bug 87155.