Implement select element list box with shadow DOM.

Implement select element list box with shadow DOM.

BUG=

[yosin_UTC9, 2013-04-17 04:00:04]

I love to use shadow DOM for "select" element.

https://codereview.chromium.org/14096013/diff/1/Source/WebCore/rendering/Rend...
File Source/WebCore/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/14096013/diff/1/Source/WebCore/rendering/Rend...
Source/WebCore/rendering/RenderListBox.cpp:110: void
RenderListBox::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop,
LogicalExtentComputedValues& computedValues) const
How about using customStyleForRenderer(), DateTimeEditElement?
And get rid of RenderListBox.cpp?

[keishi, 2013-04-17 07:18:34]

https://codereview.chromium.org/14096013/diff/1/Source/WebCore/rendering/Rend...
File Source/WebCore/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/14096013/diff/1/Source/WebCore/rendering/Rend...
Source/WebCore/rendering/RenderListBox.cpp:110: void
RenderListBox::computeLogicalHeight(LayoutUnit, LayoutUnit logicalTop,
LogicalExtentComputedValues& computedValues) const
On 2013/04/17 04:00:04, Yoshifumi Inoue wrote:
> How about using customStyleForRenderer(), DateTimeEditElement?
> And get rid of RenderListBox.cpp?

We shouldn't override the css height specified by the user. I don't think we can
do that if we use customStyleForRenderer(). The style would already be resolved
when it is called and I don't think we can tell if the height was specified in
CSS or not.

[keishi, 2013-04-17 07:27:49]

Some known issues:
- Scrolls horizontally on auto scroll.
- List box height is determined by font size. Firefox uses max(option height) *
size
- Scrollbar overlaps on Mac.

Intentional changes from the existing one:
- optgroup indentation is done with padding instead of spaces so there is a
slight shift.
- The height of the last item is one pixel short when the scroll height isn't
overflowing.
- Scroll offset after page down/up don't align with the option border
- Scroll bar is thicker on Mac

[keishi, 2013-04-17 07:31:25]

On 2013/04/17 07:27:49, keishi1 wrote:
> Intentional changes from the existing one:
Also baseline is slightly lower. I used "vertical-align: text-bottom; " which is
what Firefox uses.

[tkent, 2013-04-17 08:05:14]

You might want to update core/editing/TextIterator.* to avoid updating a lot of
layout test results.

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/css/CSSValu...
File Source/WebCore/css/CSSValueKeywords.in (right):

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/css/CSSValu...
Source/WebCore/css/CSSValueKeywords.in:139: activelistboxselection
We shouldn't expose these color names to web authors.

[ojan, 2013-04-22 07:55:56]

Just a couple drive-by comments as I happened to glance at this review.

Please give a much more detailed change description. Someone coming in and
looking at this patch 3 months from now should be able to understand why you
changed each thing the way you did. You don't need a comment for every line of
code of course, but an explanation of anything non-trivial (of which there is a
lot in this patch!).

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
File Source/WebCore/accessibility/AccessibilityListBoxOption.cpp (right):

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
Source/WebCore/accessibility/AccessibilityListBoxOption.cpp:109: //   
LayoutRect parentRect =
listBoxRenderer->document()->axObjectCache()->getOrCreate(listBoxRenderer)->boundingBoxRect();
Was this intentional? We should never commit commented out code. If the code
doesn't belong anymore, just delete it. Same elsewhere.

[keishi, 2013-04-23 02:06:03]

On 2013/04/22 07:55:56, ojan wrote:
> Just a couple drive-by comments as I happened to glance at this review.
> 
> Please give a much more detailed change description. Someone coming in and
> looking at this patch 3 months from now should be able to understand why you
> changed each thing the way you did. You don't need a comment for every line of
> code of course, but an explanation of anything non-trivial (of which there is
a
> lot in this patch!).

Sorry, I will update the description with more detail.

>
https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
> File Source/WebCore/accessibility/AccessibilityListBoxOption.cpp (right):
> 
>
https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
> Source/WebCore/accessibility/AccessibilityListBoxOption.cpp:109: //   
> LayoutRect parentRect =
>
listBoxRenderer->document()->axObjectCache()->getOrCreate(listBoxRenderer)->boundingBoxRect();
> Was this intentional? We should never commit commented out code. If the code
> doesn't belong anymore, just delete it. Same elsewhere.

I should have been more clear that it is a WIP patch. I will fix it in my next
patch.

[yosin_UTC9, 2013-04-23 03:43:25]

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
File Source/WebCore/accessibility/AccessibilityListBox.cpp (right):

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
Source/WebCore/accessibility/AccessibilityListBox.cpp:133: //    unsigned length
= m_children.size();
You may forget to remove this fragment.

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/accessibili...
Source/WebCore/accessibility/AccessibilityListBox.cpp:163: //    LayoutRect
parentRect = boundingBoxRect();
You may forget to remove this fragment.

[keishi, 2013-04-23 09:42:51]

This is blocked by support for user agent stylesheet only CSS values.
https://code.google.com/p/chromium/issues/detail?id=234270

[yosin_UTC9, 2013-05-01 01:38:21]

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/html/HTMLOp...
File Source/WebCore/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/html/HTMLOp...
Source/WebCore/html/HTMLOptionElement.cpp:70:
option->ensureUserAgentShadowRoot();
Can we postpone creating shadow root until this option element  in listbox? We
may not want to have shadow root for datalist/option.

https://codereview.chromium.org/14096013/diff/9008/Source/WebCore/html/HTMLOp...
Source/WebCore/html/HTMLOptionElement.cpp:370: if (items[items.size() - 1] ==
this)
nit: We should check item.size() > 0.

[tkent, 2014-09-17 05:22:36]

This CL must be obsoleted.  Closing.