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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1633553002: Change in focus with movement of Arrow keys for RTL radio buttons (Closed)

Created:
4 years, 11 months ago by chakshu
Modified:
4 years, 10 months ago
Reviewers:
tkent, pals, vivekg_samsung
CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Change in focus with movement of Arrow keys for RTL radio buttons. Updated the function findNextFocusableRadioButtonInGroup to take into consideration the computedTextDirection for deciding which radio button to move the focus on. BUG=556677 Committed: https://crrev.com/f32d1e0861a94a3091f52a5f4b226e0b507d6dad Cr-Commit-Position: refs/heads/master@{#374652}

Patch Set 1 #

Total comments: 6

Patch Set 2 : Made changes according to review comments #

Total comments: 8

Patch Set 3 : Removed the use of non-ASCII characters. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -1 line) Patch
M third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html View 1 2 2 chunks +28 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event-expected.txt View 1 2 2 chunks +14 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/forms/RadioInputType.cpp View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 18 (8 generated)
pals
looks good.
4 years, 10 months ago (2016-01-28 07:27:14 UTC) #5
chakshu
@tkent Can you look at this fix for change in focus in case of RTL ...
4 years, 10 months ago (2016-02-03 12:24:31 UTC) #7
tkent
https://codereview.chromium.org/1633553002/diff/1/third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html File third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html (right): https://codereview.chromium.org/1633553002/diff/1/third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html#newcode1 third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html:1: <!DOCTYPE html> Probably you copied radio-group-keyboard-change-event.html. Please add new ...
4 years, 10 months ago (2016-02-03 23:45:37 UTC) #8
chakshu
Included changes https://codereview.chromium.org/1633553002/diff/1/third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html File third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html (right): https://codereview.chromium.org/1633553002/diff/1/third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html#newcode1 third_party/WebKit/LayoutTests/fast/forms/radio/rtl-radio-buttons-key-event.html:1: <!DOCTYPE html> On 2016/02/03 23:45:37, tkent wrote: ...
4 years, 10 months ago (2016-02-08 09:46:40 UTC) #9
tkent
C++ code change looks ok. https://codereview.chromium.org/1633553002/diff/20001/third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html File third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html (right): https://codereview.chromium.org/1633553002/diff/20001/third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html#newcode4 third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html:4: <meta charset="UTF-16"> This file ...
4 years, 10 months ago (2016-02-08 23:32:26 UTC) #10
chakshu
Made changes as per review comments https://codereview.chromium.org/1633553002/diff/20001/third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html File third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html (right): https://codereview.chromium.org/1633553002/diff/20001/third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html#newcode4 third_party/WebKit/LayoutTests/fast/forms/radio/radio-group-keyboard-change-event.html:4: <meta charset="UTF-16"> On ...
4 years, 10 months ago (2016-02-10 10:17:02 UTC) #11
tkent
lgtm
4 years, 10 months ago (2016-02-10 10:19:01 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1633553002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1633553002/40001
4 years, 10 months ago (2016-02-10 10:19:15 UTC) #14
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 10 months ago (2016-02-10 13:29:23 UTC) #16
commit-bot: I haz the power
4 years, 10 months ago (2016-02-10 13:31:43 UTC) #18
Message was sent while issue was closed. 
Patchset 3 (id:??) landed as
https://crrev.com/f32d1e0861a94a3091f52a5f4b226e0b507d6dad
Cr-Commit-Position: refs/heads/master@{#374652}

Powered by Google App Engine
This is Rietveld 408576698