Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5)

Issue 1186623002: This test needs to use EventSender instead of initMouseEvent so that the pending fix for 423975 can… (Closed)

Created:
5 years, 6 months ago by kirkshoop_msft
Modified:
4 years, 7 months ago
Reviewers:
Rick Byers
CC:
chromium-reviews, plundblad+watch_chromium.org, aboxhall+watch_chromium.org, nektar+watch_chromium.org, jam, yuzo+watch_chromium.org, je_julie, darin-cc_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, dmazzoni
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Fix test generating synthetic mouse events to use EventSender See https://codereview.chromium.org/894913002/ for details. BUG=423975 R=rbyers@chromium.org

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M content/test/data/accessibility/event/menulist-popup.html View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
Rick Byers
LGTM
5 years, 6 months ago (2015-06-12 20:01:32 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1186623002/1
5 years, 6 months ago (2015-06-12 20:22:52 UTC) #3
Rick Byers
On 2015/06/12 20:01:32, Rick Byers wrote: > LGTM By the way, I'm kind of surprised ...
5 years, 6 months ago (2015-06-12 20:59:04 UTC) #4
kirkshoop_msft
On 2015/06/12 20:59:04, Rick Byers wrote: > On 2015/06/12 20:01:32, Rick Byers wrote: > > ...
5 years, 6 months ago (2015-06-12 21:13:55 UTC) #5
Rick Byers
On 2015/06/12 21:13:55, kirkshoop_msft wrote: > On 2015/06/12 20:59:04, Rick Byers wrote: > > On ...
5 years, 6 months ago (2015-06-12 21:25:19 UTC) #7
kirkshoop_msft
On 2015/06/12 21:25:19, Rick Byers (Slow until 6-22) wrote: > On 2015/06/12 21:13:55, kirkshoop_msft wrote: ...
5 years, 6 months ago (2015-06-15 16:14:40 UTC) #8
Rick Byers
5 years, 6 months ago (2015-06-15 16:31:25 UTC) #9
On 2015/06/15 16:14:40, kirkshoop_msft wrote:
> On 2015/06/12 21:25:19, Rick Byers (Slow until 6-22) wrote:
> > On 2015/06/12 21:13:55, kirkshoop_msft wrote:
> > > According to CQ, you are correct. The eventSender is not available. I
could
> go
> > > back to using initMouseEvent but change mousedown to click, or something
> else.
> > > 
> > > Any thoughts?
> > 
> > If click produces the same output, then yes that sounds ideal.  Otherwise
> we'll
> > need to somehow give this test the ability to synthesize a real input event.

> > +dmazzoni who owns these tests.
> > 
> 
> I cannot get click or keyboard events to trigger the select popup with
> https://codereview.chromium.org/894913002 applied.
> 
> I think that eventSender will have to be made available to the accessibility
> tests.
> 
> There is also this answer that will need to be updated. 
>   http://stackoverflow.com/a/20906852 
> In fact, this may be the largest compatibility impact, there appears to be no
> other way to programmatically cause a select to open.

Yep, but since that's the ultimate goal of your bug (to bring Chrome into
alignment with IE and Firefox in this regard) it's the fundamental trade-off.

One option to move this forward in a more incremental fashion would be to
propose a new API for programatically opening a select box.  It does seem like
there's need for such an API.  Do you want to see if anyone on the IE team is
interested in trying to standardize such a simple API with us?

That said I wouldn't want to block this CL.  What would you think about adding a
special case to your CL to allow mousedown on <select> to continue to behave as
before.  Then at least we can land all the other benefits here and focus on the
primary scenario in a follow-up CL?  I'm still OK with trying to change this
behavior (even in advance of having an alternative), but it's entirely possible
we'd have to revert due to too many sites depending on this in WebKit browsers. 
So it would be helpful if we could separate all the infrastructure work you've
done here with the main breaking behavior change.  WDYT?

Powered by Google App Engine
This is Rietveld 408576698