Update <select> when any of its <option> children has "display: none"

Update <select> when any of its <option>/<optgroup> children have "display: none"

The <select> element does not ignore the <option>/<optgroup> children that have "display:none". Conseqently, these children are displayed on the list. Code has been added to detect the change in display property and also to drop <option>/<optgroup> children whenever required.

BUG=81415
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=171457

[spartha, 2014-03-13 13:55:52]

Proposed Patch PTAL....

[tkent, 2014-03-13 23:39:05]

What will the following code show with this patch?

<select>
 <option>Option 0</option>
 <option style="display:none;">Option 1</option>
 <option>Option 2</option>
</select>
<script>
var s = document.querySelector('select');
s.selectedIndex = 1;
alert(s.value);
</script>

[spartha, 2014-03-14 05:11:03]

With the patch the alert prints "Option 2". The behavior is different with
FireFox and Chrome(without patch). They both print "Option 1". This behaviour is
urprising. Is there a specification that prescribes this?

On 2014/03/13 23:39:05, tkent wrote:
> What will the following code show with this patch?
> 
> <select>
>  <option>Option 0</option>
>  <option style="display:none;">Option 1</option>
>  <option>Option 2</option>
> </select>
> <script>
> var s = document.querySelector('select');
> s.selectedIndex = 1;
> alert(s.value);
> </script>

[tkent, 2014-03-14 05:19:12]

On 2014/03/14 05:11:03, spartha wrote:
> With the patch the alert prints "Option 2". The behavior is different with
> FireFox and Chrome(without patch). They both print "Option 1". This behaviour
is
> urprising. Is there a specification that prescribes this?

There is no standard about this specific behavior.  But the principle is that
CSS style must not affect DOM API behavior. We should not modify m_listItems to
fix this bug.

[spartha, 2014-03-14 05:28:38]

On 2014/03/14 05:11:03, spartha wrote:

> With the patch the alert prints "Option 2". The behavior is different with
> FireFox and Chrome(without patch). They both print "Option 1". This behaviour
is
> urprising. Is there a specification that prescribes this?
> 
> On 2014/03/13 23:39:05, tkent wrote:
> > What will the following code show with this patch?
> > 
> > <select>
> >  <option>Option 0</option>
> >  <option style="display:none;">Option 1</option>
> >  <option>Option 2</option>
> > </select>
> > <script>
> > var s = document.querySelector('select');
> > s.selectedIndex = 1;
> > alert(s.value);
> > </script>
I see there are quite a few other issues with this patch. Atleast on FF, DOM
APIs like "namedItem" don't care about the display property. They still return
the element when queried.

<select>
 <option >Option 0</option>
 <option name="o1" style="display:none">Option 1</option>
 <option>Option 2</option>
</select>
<script>
    var s = document.querySelector('select');
    var name = s.namedItem('o1');
    alert(name.value);
</script>
Not adding the items to the list is *Not* a good idea :( 
Is there a better way to do this?

[spartha, 2014-03-18 19:11:58]

Uploaded 2nd WIP Patch...PTAL

[tkent, 2014-03-20 02:29:50]

-eseidel, +keishi

- Please add tests
- What about menulist SELECT?

[spartha, 2014-03-20 04:58:19]

On 2014/03/20 02:29:50, tkent wrote:
> -eseidel, +keishi
> 
> - Please add tests
Sure

> - What about menulist SELECT?
menulist select takes a different path. So it does not have the same issues seen
with RenderListBox. Most of the logic to handle the changes are implemented in
PopupListBox.

[spartha, 2014-03-20 10:00:07]

Updated the patch, PTAL, Thanks!

[esprehn, 2014-03-20 17:09:25]

I'm not sure this complexity is worth it. We intentionally don't support
display: none on <option> and the spec is silent on it.

You definitely can't use a setTimeout(0) though.

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLOp...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:310: select->updateListOnRenderer();
This is not the correct place to do this.

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:386: if (style && style->display() ==
NONE)
return style && style->display() == NONE;

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.cpp:1634:
m_optionChildChangedTimer.startOneShot(0, FROM_HERE);
This is not correct, you can't use a timer.

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
File Source/core/html/HTMLSelectElement.h (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.h:215: Timer<HTMLSelectElement>
m_optionChildChangedTimer;
Please remove this timer, it's not correct to use one and it creates a race.

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:132: if (listItemStyle &&
listItemStyle->display() == NONE)
isDisplayNone() ? Why do you need to check it both here and in the other loops?

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.h (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.h:157: Vector<int> m_listItemIndex;
This doesn't seem right.

[spartha, 2014-03-21 05:13:25]

Thanks for the review! I see that the select that uses the menulist, handles the
cases with display none. The motivation for this change is uniformity in
behavior.

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLOp...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:310: select->updateListOnRenderer();
On 2014/03/20 17:09:25, esprehn wrote:
> This is not the correct place to do this.

Since we don't have a renderer associated with this element, we cannot rely on
styleDidChange. I was looking for the change in display property, so added the
check here. Is there a better place to add this check?

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:386: if (style && style->display() ==
NONE)
On 2014/03/20 17:09:25, esprehn wrote:
> return style && style->display() == NONE;

Done.

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
File Source/core/html/HTMLSelectElement.h (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.h:215: Timer<HTMLSelectElement>
m_optionChildChangedTimer;
On 2014/03/20 17:09:25, esprehn wrote:
> Please remove this timer, it's not correct to use one and it creates a race.
The idea to use the timer is to avoid multiple calls to updateFromElement. I can
think of using a boolean switch on select instead. The Option elements can set a
flag on select when there is a change in display. At the end of style
recalculation I can call updateFromElement if required.

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:132: if (listItemStyle &&
listItemStyle->display() == NONE)
On 2014/03/20 17:09:25, esprehn wrote:
> isDisplayNone() ? Why do you need to check it both here and in the other
loops?
I presume the other loops you are referring to are the ones in nextValidIndex &
updateListBoxSelection. They are needed to skip selecting the items that are not
displayed. The check here, is needed to skip drawing the item on screen.

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.h (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.h:157: Vector<int> m_listItemIndex;
On 2014/03/20 17:09:25, esprehn wrote:
> This doesn't seem right.
Since modifying the item list on the select element is not an option here, I
thought the best way to go about is to have a separate list maintained by the
renderer.

[esprehn, 2014-03-21 20:43:38]

https://codereview.chromium.org/189543012/diff/140001/Source/core/html/HTMLOp...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/189543012/diff/140001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:308: if ((m_style->display() == NONE ||
(!oldStyle || oldStyle->display() == NONE)) && (!oldStyle || (m_style->display()
!= oldStyle->display()))) {
This can be much simpler, just do:

bool wasDisplayNone = isDisplayNone();
updateNonRenderStyle();
if (isDisplayNone() != wasDisplayNone) {
  ...
}

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:212: int lastIndex =
static_cast<int>(m_listItemIndex.find(select->activeSelectionEndListIndex()));
Why can't we just iterate the set of items and only consider the visible ones?

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:361: HTMLElement* element =
listItems[m_listItemIndex[i]];
ditto.

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:726: for (int i = 0; i < size; ++i) {
Since we're walking all of them anyway, why can't we just skip display: none
ones?

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.h (right):

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.h:157: Vector<int> m_listItemIndex;
It seems unfortunate to keep this Vector the size of the options around when
almost no real pages use display: none, so this is just wasting memory.

[spartha, 2014-03-21 21:29:02]

https://codereview.chromium.org/189543012/diff/140001/Source/core/html/HTMLOp...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/189543012/diff/140001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:308: if ((m_style->display() == NONE ||
(!oldStyle || oldStyle->display() == NONE)) && (!oldStyle || (m_style->display()
!= oldStyle->display()))) {
On 2014/03/21 20:43:39, esprehn wrote:
> This can be much simpler, just do:
> 
> bool wasDisplayNone = isDisplayNone();
> updateNonRenderStyle();
> if (isDisplayNone() != wasDisplayNone) {
>   ...
> }

Done.

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:212: int lastIndex =
static_cast<int>(m_listItemIndex.find(select->activeSelectionEndListIndex()));
On 2014/03/21 20:43:39, esprehn wrote:
> Why can't we just iterate the set of items and only consider the visible ones?

As against using the vector? We can do that I suppose. But then we would have to
loop through the item list each time we need to get the element from the display
order (which may not be the same as the item list in Select). It comes down to
the choice between speed v/s space. I have yet to come across select with huge
number of items. So if space is a concern I can try the other approach.
Thoughts?

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.h (right):

https://codereview.chromium.org/189543012/diff/140001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.h:157: Vector<int> m_listItemIndex;
On 2014/03/21 20:43:39, esprehn wrote:
> It seems unfortunate to keep this Vector the size of the options around when
> almost no real pages use display: none, so this is just wasting memory.
Yeah, totally agree. The only reason I want to use it, is to keep the code
simple.

[spartha, 2014-03-21 21:37:36]

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.cpp:1634:
m_optionChildChangedTimer.startOneShot(0, FROM_HERE);
On 2014/03/20 17:09:25, esprehn wrote:
> This is not correct, you can't use a timer.
Just curious, when is it recommended to use a timer?

[esprehn, 2014-03-24 09:16:34]

On 2014/03/21 at 21:37:36, sudarshan.p wrote:
>
https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
> File Source/core/html/HTMLSelectElement.cpp (right):
> 
>
https://codereview.chromium.org/189543012/diff/100001/Source/core/html/HTMLSe...
> Source/core/html/HTMLSelectElement.cpp:1634:
m_optionChildChangedTimer.startOneShot(0, FROM_HERE);
> On 2014/03/20 17:09:25, esprehn wrote:
> > This is not correct, you can't use a timer.
> Just curious, when is it recommended to use a timer?

When you don't care about if/when the task happens and there's no ordering
dependency on the operation.

In this case failing to perform the update means we'd paint the wrong thing for
the next frame. In fact, if there's lots of other setTimeout(0) work to be done
we could paint the wrong thing for many frames before this timer finally fired
and then suddenly the items would disappear for the user.

[tkent, 2014-03-24 09:19:42]

I remember Keishi's on-going patch <https://codereview.chromium.org/14096013/>,
and think Keishi's will resolve crbug.com/81415 too.  Keishi, what do you think
about this CL?

[spartha, 2014-03-31 15:33:04]

On 2014/03/24 09:19:42, tkent wrote:
> I remember Keishi's on-going patch
<https://codereview.chromium.org/14096013/>,
> and think Keishi's will resolve crbug.com/81415 too.  Keishi, what do you
think
> about this CL?

I can close this review if the patch is not relevant. Is this patch going to
land soon? I have another of my patches that depends on "UA only" properties
like the one above.

[keishi, 2014-04-11 06:40:10]

Sorry for being late. Hopefully we can move <select> to shadow dom soon, but I
think we can land this.

https://codereview.chromium.org/189543012/diff/180001/Source/core/accessibili...
File Source/core/accessibility/AXListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/180001/Source/core/accessibili...
Source/core/accessibility/AXListBox.cpp:155: LayoutRect rect =
listBox->itemBoundingBoxRect(parentRect.location(),
listBox->toRenderListBoxIndex(i));
Ditto. toRenderListBoxIndex should be done inside RenderListBox.

https://codereview.chromium.org/189543012/diff/180001/Source/core/accessibili...
File Source/core/accessibility/AXListBoxOption.cpp (right):

https://codereview.chromium.org/189543012/diff/180001/Source/core/accessibili...
Source/core/accessibility/AXListBoxOption.cpp:110: rect =
listBox->itemBoundingBoxRect(parentRect.location(),
listBox->toRenderListBoxIndex(index));
Ditto. toRenderListBoxIndex should be done inside RenderListBox.

https://codereview.chromium.org/189543012/diff/180001/Source/core/html/HTMLSe...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/189543012/diff/180001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.cpp:850:
listBox->scrollToRevealElementAtListIndex(listBox->toRenderListBoxIndex(suggestedIndex));
I don't like to have yet another index. We have optionIndex and listIndex.
scrollToRevealElementAtListIndex should take a listIndex.
I think renderListBoxIndex should only be used inside RenderListBox.

https://codereview.chromium.org/189543012/diff/180001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.cpp:1458:
toRenderListBox(renderer())->scrollToRevealElementAtListIndex(listBox->toRenderListBoxIndex(endIndex));
Ditto.

https://codereview.chromium.org/189543012/diff/180001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/180001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:960: int
RenderListBox::toOptionListIndex(int index) const
We have lots of indexes so it would be great if we could tell what index we are
converting from. e.g. renderListBoxIndexToOptionIndex

[spartha, 2014-04-11 11:19:44]

Thanks for the review! Have updated the patch as per the review comments. PTAL

https://codereview.chromium.org/189543012/diff/180001/Source/core/html/HTMLSe...
File Source/core/html/HTMLSelectElement.cpp (right):

https://codereview.chromium.org/189543012/diff/180001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.cpp:850:
listBox->scrollToRevealElementAtListIndex(listBox->toRenderListBoxIndex(suggestedIndex));
On 2014/04/11 06:40:13, keishi1 wrote:
> I don't like to have yet another index. We have optionIndex and listIndex.
> scrollToRevealElementAtListIndex should take a listIndex.
> I think renderListBoxIndex should only be used inside RenderListBox.

Done.

https://codereview.chromium.org/189543012/diff/180001/Source/core/html/HTMLSe...
Source/core/html/HTMLSelectElement.cpp:1458:
toRenderListBox(renderer())->scrollToRevealElementAtListIndex(listBox->toRenderListBoxIndex(endIndex));
On 2014/04/11 06:40:13, keishi1 wrote:
> Ditto.

Done.

https://codereview.chromium.org/189543012/diff/180001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/180001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:960: int
RenderListBox::toOptionListIndex(int index) const
On 2014/04/11 06:40:13, keishi1 wrote:
> We have lots of indexes so it would be great if we could tell what index we
are
> converting from. e.g. renderListBoxIndexToOptionIndex

Done.

[keishi, 2014-04-11 15:07:09]

https://codereview.chromium.org/189543012/diff/200001/LayoutTests/fast/forms/...
File LayoutTests/fast/forms/select/listbox-with-display-none-option.html
(right):

https://codereview.chromium.org/189543012/diff/200001/LayoutTests/fast/forms/...
LayoutTests/fast/forms/select/listbox-with-display-none-option.html:1: <!doctype
html>
We should add tests for
- changing style from js and see if the appearance is updated.
- display:none for optgroup

https://codereview.chromium.org/189543012/diff/200001/Source/core/html/HTMLOp...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/189543012/diff/200001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:1: /*
We need to do the same changes to HTMLOptGroupElement.

HTMLHRElement is non standard so I think we should just ignore it for this
patch.

https://codereview.chromium.org/189543012/diff/200001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:375: return style && style->display() ==
NONE;
We should check parent optgroup.isDisplayNone

https://codereview.chromium.org/189543012/diff/200001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/200001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:963: int
RenderListBox::renderListBoxIndexToOptionIndex(int index) const
I'm sorry this should have been renderListBoxIndexToListIndex.
listIndex is the index used for HTMLSelectItem::listItems() which contains
option element, optgroup elements, and hr elements.
optionIndex is the index for option elements only.

https://codereview.chromium.org/189543012/diff/200001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:985: int
RenderListBox::optionIndexToRenderListBoxIndex(int index) const
Ditto.

[spartha, 2014-04-12 18:53:22]

Done. PTAL

https://codereview.chromium.org/189543012/diff/200001/LayoutTests/fast/forms/...
File LayoutTests/fast/forms/select/listbox-with-display-none-option.html
(right):

https://codereview.chromium.org/189543012/diff/200001/LayoutTests/fast/forms/...
LayoutTests/fast/forms/select/listbox-with-display-none-option.html:1: <!doctype
html>
On 2014/04/11 15:07:09, keishi1 wrote:
> We should add tests for
> - changing style from js and see if the appearance is updated.
> - display:none for optgroup

Done.

https://codereview.chromium.org/189543012/diff/200001/Source/core/html/HTMLOp...
File Source/core/html/HTMLOptionElement.cpp (right):

https://codereview.chromium.org/189543012/diff/200001/Source/core/html/HTMLOp...
Source/core/html/HTMLOptionElement.cpp:375: return style && style->display() ==
NONE;
On 2014/04/11 15:07:09, keishi1 wrote:
> We should check parent optgroup.isDisplayNone

Done.

https://codereview.chromium.org/189543012/diff/200001/Source/core/rendering/R...
File Source/core/rendering/RenderListBox.cpp (right):

https://codereview.chromium.org/189543012/diff/200001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:963: int
RenderListBox::renderListBoxIndexToOptionIndex(int index) const
On 2014/04/11 15:07:09, keishi1 wrote:
> I'm sorry this should have been renderListBoxIndexToListIndex.
> listIndex is the index used for HTMLSelectItem::listItems() which contains
> option element, optgroup elements, and hr elements.
> optionIndex is the index for option elements only.

Done.

https://codereview.chromium.org/189543012/diff/200001/Source/core/rendering/R...
Source/core/rendering/RenderListBox.cpp:985: int
RenderListBox::optionIndexToRenderListBoxIndex(int index) const
On 2014/04/11 15:07:09, keishi1 wrote:
> Ditto.

Done.

[keishi, 2014-04-13 03:31:20]

LGTM. Please add something about optgroup to the change description.

[spartha, 2014-04-14 05:40:25]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 05:40:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/220001

[commit-bot: I haz the power, 2014-04-14 06:10:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 06:10:06]

Try jobs failed on following builders:
  tryserver.blink on mac_blink_compile_dbg

[spartha, 2014-04-14 08:53:05]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 08:53:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/230001

[commit-bot: I haz the power, 2014-04-14 09:22:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 09:22:11]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_rel

[spartha, 2014-04-14 09:45:55]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 09:46:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/250001

[commit-bot: I haz the power, 2014-04-14 10:19:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 10:19:34]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[spartha, 2014-04-14 10:30:38]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 10:30:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/270001

[commit-bot: I haz the power, 2014-04-14 10:30:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 10:30:56]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1080.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
e0996a0ab263bab69a5ce6cf68aee638e0acec7b..e6caf36d089620a492584ebc84392d48371b99ba
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1080,3 +1080,5 @@ crbug.com/356780 [ Win ]
virtual/windows-directwrite/fast/text/textIteratorNilRe
 
 crbug.com/362434 [ Mac ]
compositing/overflow/do-not-paint-outline-into-composited-scrolling-contents.html
[ ImageOnlyFailure Pass ]
 crbug.com/362434 [ Mac ]
virtual/gpu/compositedscrolling/overflow/do-not-paint-outline-into-composited-scrolling-contents.html
[ ImageOnlyFailure Pass ]
+
+crbug.com/81415 [ Win Mac ]
LayoutTests/fast/forms/select/listbox-with-display-none-option.html

[spartha, 2014-04-14 10:31:31]

The CQ bit was unchecked by sudarshan.p@samsung.com

[spartha, 2014-04-14 10:33:53]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 10:34:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/290001

[commit-bot: I haz the power, 2014-04-14 10:34:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 10:34:16]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1080.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
e0996a0ab263bab69a5ce6cf68aee638e0acec7b..ca8ba760ff49a2c2e17220c586c7c3f377e99736
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1080,3 +1080,5 @@ crbug.com/356780 [ Win ]
virtual/windows-directwrite/fast/text/textIteratorNilRe
 
 crbug.com/362434 [ Mac ]
compositing/overflow/do-not-paint-outline-into-composited-scrolling-contents.html
[ ImageOnlyFailure Pass ]
 crbug.com/362434 [ Mac ]
virtual/gpu/compositedscrolling/overflow/do-not-paint-outline-into-composited-scrolling-contents.html
[ ImageOnlyFailure Pass ]
+
+crbug.com/81415 [ Win Mac ]
fast/forms/select/listbox-with-display-none-option.html [ NeedsRebaseline ]

[spartha, 2014-04-14 10:35:08]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 10:35:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/290001

[commit-bot: I haz the power, 2014-04-14 10:35:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 10:35:23]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 1080.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
e0996a0ab263bab69a5ce6cf68aee638e0acec7b..ca8ba760ff49a2c2e17220c586c7c3f377e99736
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -1080,3 +1080,5 @@ crbug.com/356780 [ Win ]
virtual/windows-directwrite/fast/text/textIteratorNilRe
 
 crbug.com/362434 [ Mac ]
compositing/overflow/do-not-paint-outline-into-composited-scrolling-contents.html
[ ImageOnlyFailure Pass ]
 crbug.com/362434 [ Mac ]
virtual/gpu/compositedscrolling/overflow/do-not-paint-outline-into-composited-scrolling-contents.html
[ ImageOnlyFailure Pass ]
+
+crbug.com/81415 [ Win Mac ]
fast/forms/select/listbox-with-display-none-option.html [ NeedsRebaseline ]

[spartha, 2014-04-14 10:42:05]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 10:42:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/310001

[commit-bot: I haz the power, 2014-04-14 11:13:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 11:13:55]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_rel

[spartha, 2014-04-14 11:39:26]

The CQ bit was checked by sudarshan.p@samsung.com

[commit-bot: I haz the power, 2014-04-14 11:39:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sudarshan.p@samsung.com/189543012/330001

[commit-bot: I haz the power, 2014-04-14 12:45:08]

Change committed as 171457