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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1312093002: Set focused flag earlier than dispatching focus events. (Closed)

Created:
5 years, 4 months ago by kochi
Modified:
5 years, 4 months ago
Reviewers:
esprehn
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Target Ref:
refs/remotes/origin/master
Project:
blink
Visibility:
Public.

Description

Set focused flag earlier than dispatching focus events. Blink used to set focused flag on focused element after dispatching focus events. This caused mismatch with other browsers, during event handler document.querySelector(":focus") doesn't match anything. BUG=523126 TEST=fast/events/focus-querySelector-in-focus-event-handler.html Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201118

Patch Set 1 #

Patch Set 2 : layout test only #

Patch Set 3 : run with the fix #

Total comments: 2

Patch Set 4 : remove body tag. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+18 lines, -4 lines) Patch
A LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html View 1 2 3 1 chunk +15 lines, -0 lines 0 comments Download
A + LayoutTests/fast/events/focus-querySelector-in-focus-event-handler-expected.txt View 1 1 chunk +1 line, -2 lines 0 comments Download
M Source/core/dom/Document.cpp View 2 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 13 (6 generated)
kochi
PTAL
5 years, 4 months ago (2015-08-25 07:44:43 UTC) #3
esprehn
lgtm https://codereview.chromium.org/1312093002/diff/60001/LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html File LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html (right): https://codereview.chromium.org/1312093002/diff/60001/LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html#newcode3 LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html:3: <body> I don't think you need the <body>
5 years, 4 months ago (2015-08-25 07:50:19 UTC) #4
kochi
Thanks for the review! <body> tag removed. https://codereview.chromium.org/1312093002/diff/60001/LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html File LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html (right): https://codereview.chromium.org/1312093002/diff/60001/LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html#newcode3 LayoutTests/fast/events/focus-querySelector-in-focus-event-handler.html:3: <body> On ...
5 years, 4 months ago (2015-08-25 08:04:34 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1312093002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1312093002/80001
5 years, 4 months ago (2015-08-25 08:05:02 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/103701)
5 years, 4 months ago (2015-08-25 08:57:19 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1312093002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1312093002/80001
5 years, 4 months ago (2015-08-25 08:58:39 UTC) #12
commit-bot: I haz the power
5 years, 4 months ago (2015-08-25 10:06:35 UTC) #13
Message was sent while issue was closed. 
Committed patchset #4 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201118

Powered by Google App Engine
This is Rietveld 408576698