Revert of Add AX role ,'AXDescriptionList', for dl element.

Revert of Add AX role ,'AXDescriptionList', for dl element. (patchset #2 id:20001 of https://codereview.chromium.org/609793002/)

Reason for revert:
This patch, probably, broke AccesibilityDl test on Mac
http://build.chromium.org/p/chromium.webkit/builders/Win7%20Tests/builds/5898

Original issue's description:
> Add AX role ,'AXDescriptionList', for dl element.
> 
> There are related patches from WebKit, the patch written by
> James Craig<james@cookiecrook.com>; wkrev.com/147520 and the
> patch written by Mario Sanchez Prada  <mario.prada@samsung.com>;
> wkrev.com/161492.
> And I modified them based on the current blink.
> 
> BUG=226505
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182813

TBR=dmazzoni@chromium.org,je_julie.kim@samsung.com
NOTREECHECKS=true
NOTRY=true
BUG=226505
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182847

[eustas, 2014-09-29 15:29:33]

Created Revert of Add AX role ,'AXDescriptionList', for dl element.

[commit-bot: I haz the power, 2014-09-29 15:30:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/616553002/1

[commit-bot: I haz the power, 2014-09-29 15:31:04]

Committed patchset #1 (id:1) as 182847

[je_julie(Not used), 2014-09-30 00:10:08]

On 2014/09/29 15:31:04, I haz the power (commit-bot) wrote:
> Committed patchset #1 (id:1) as 182847

Hi eustas,
I saw the log for content_browsertests.
But I have the related patch,https://codereview.chromium.org/607053002/, to
this.

When we add AX role, patch for blink is first and then patch for chromium.
There could be gap between blink and chromium when patch for blink is applied.
So far, I think it's avoidable because repository is different.
I checked already All Accesibility Tests are passed with blink patch and
chromium patch.

For this situation, is there any other suggestion?

[dmazzoni, 2014-09-30 06:48:26]

On 2014/09/30 00:10:08, je_julie.kim wrote:
> On 2014/09/29 15:31:04, I haz the power (commit-bot) wrote:
> > Committed patchset #1 (id:1) as 182847
> 
> Hi eustas,
> I saw the log for content_browsertests.
> But I have the related patch,https://codereview.chromium.org/607053002/, to
> this.
> 
> When we add AX role, patch for blink is first and then patch for chromium.
> There could be gap between blink and chromium when patch for blink is applied.
> So far, I think it's avoidable because repository is different.
> I checked already All Accesibility Tests are passed with blink patch and
> chromium patch.
> 
> For this situation, is there any other suggestion?

Sadly, the only solution is what you're doing - disable the test, land the Blink
change, then fix the tests in Chromium.

It's a mess, I can't wait until the repositories are merged.

In the future, you don't have to create a new issue to re-land a change I
already LGTM'd - just edit it, uncheck "closed" to reopen it, then make a
comment explaining what you're doing and press the commit box again.

[je_julie(Not used), 2014-09-30 08:25:21]

On 2014/09/30 06:48:26, dmazzoni wrote:
> On 2014/09/30 00:10:08, je_julie.kim wrote:
> > On 2014/09/29 15:31:04, I haz the power (commit-bot) wrote:
> > > Committed patchset #1 (id:1) as 182847
> > 
> > Hi eustas,
> > I saw the log for content_browsertests.
> > But I have the related patch,https://codereview.chromium.org/607053002/, to
> > this.
> > 
> > When we add AX role, patch for blink is first and then patch for chromium.
> > There could be gap between blink and chromium when patch for blink is
applied.
> > So far, I think it's avoidable because repository is different.
> > I checked already All Accesibility Tests are passed with blink patch and
> > chromium patch.
> > 
> > For this situation, is there any other suggestion?
> 
> Sadly, the only solution is what you're doing - disable the test, land the
Blink
> change, then fix the tests in Chromium.
> 
> It's a mess, I can't wait until the repositories are merged.
> 
> In the future, you don't have to create a new issue to re-land a change I
> already LGTM'd - just edit it, uncheck "closed" to reopen it, then make a
> comment explaining what you're doing and press the commit box again.

Hi Dominic,
For this time, I'll upload a new CL like,
1) Skip some content_browsertests failed for that patch,'Add AX role
,'AXDescriptionList', for dl element'.(chromium).
2) commit blink patch,'Add AX role ,'AXDescriptionList', for dl element'.
(blink) -> I'll use checkboxes for "closed" and "commit"
3) after merged blink patch, chromium patch will be reviewed and merged
(chromium)
5) Enable content_browsertests skipped for that.(chromium)
4) upload layout test for dl element. (blink)

Thank you for your help.

[je_julie(Not used), 2014-09-30 14:18:35]

On 2014/09/30 08:25:21, je_julie.kim wrote:
> On 2014/09/30 06:48:26, dmazzoni wrote:
> > On 2014/09/30 00:10:08, je_julie.kim wrote:
> > > On 2014/09/29 15:31:04, I haz the power (commit-bot) wrote:
> > > > Committed patchset #1 (id:1) as 182847
> > > 
> > > Hi eustas,
> > > I saw the log for content_browsertests.
> > > But I have the related patch,https://codereview.chromium.org/607053002/,
to
> > > this.
> > > 
> > > When we add AX role, patch for blink is first and then patch for chromium.
> > > There could be gap between blink and chromium when patch for blink is
> applied.
> > > So far, I think it's avoidable because repository is different.
> > > I checked already All Accesibility Tests are passed with blink patch and
> > > chromium patch.
> > > 
> > > For this situation, is there any other suggestion?
> > 
> > Sadly, the only solution is what you're doing - disable the test, land the
> Blink
> > change, then fix the tests in Chromium.
> > 
> > It's a mess, I can't wait until the repositories are merged.
> > 
> > In the future, you don't have to create a new issue to re-land a change I
> > already LGTM'd - just edit it, uncheck "closed" to reopen it, then make a
> > comment explaining what you're doing and press the commit box again.
> 
> Hi Dominic,
> For this time, I'll upload a new CL like,
> 1) Skip some content_browsertests failed for that patch,'Add AX role
> ,'AXDescriptionList', for dl element'.(chromium).
> 2) commit blink patch,'Add AX role ,'AXDescriptionList', for dl element'.
> (blink) -> I'll use checkboxes for "closed" and "commit"
> 3) after merged blink patch, chromium patch will be reviewed and merged
> (chromium)
> 5) Enable content_browsertests skipped for that.(chromium)
> 4) upload layout test for dl element. (blink)
> 
> Thank you for your help.

Dominic,
I saw the failed TC is already disabled at
https://code.google.com/p/chromium/issues/detail?id=418600.
On the current repo, I can't see failed TC with my patch.
But I have one concern.
When I try to apply my patch on the latest repo, I saw my patch was conflicted
with https://codereview.chromium.org/615773002/.
Can I still use checkbox for closed and commit as you told?

[dmazzoni, 2014-09-30 20:38:41]

On Tue, Sep 30, 2014 at 7:18 AM, <je_julie.kim@samsung.com> wrote:

> But I have one concern.
> When I try to apply my patch on the latest repo, I saw my patch was
> conflicted
> with https://codereview.chromium.org/615773002/.
> Can I still use checkbox for closed and commit as you told?
>

If the conflicts are trivial, you can upload a new patch and commit. Please
comment to explain what you're doing, but you don't need to wait for
another lgtm if you didn't change any code.

If you changed any actual code to resolve the conflict, please ask for
another review, or open a new changelist.

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.