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

Issue 939603002: Adding support for Smart GO NEXT feature in Android Chrome (Closed)

Created:
5 years, 10 months ago by AKV
Modified:
5 years, 4 months ago
Reviewers:
tkent, lgombos, kochi
CC:
blink-reviews, dglazkov+blink, AKVT, yosin_UTC9, bcwhite, Avi (use Gerrit), lgombos
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Adding support for Smart GO NEXT feature in Android Chrome This change takes care of providing easy navigation among text input elements inside a form. Corresponding changes to control the PREVIOUS, NEXT and GO button is done at https://codereview.chromium.org/1080693002/ BUG=410785 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=200398

Patch Set 1 #

Total comments: 20

Patch Set 2 : Fixed the review comments and reused FocusController function for Navigation. #

Total comments: 5

Patch Set 3 : Fixed the treescope problem for the current form. #

Total comments: 2

Patch Set 4 : Breaking the traversal when reaches outside current form scope. #

Total comments: 22

Patch Set 5 : Fixed the review comments by taking into consideration of input text elements outside of form, but … #

Patch Set 6 : Fixed the naming issue in Keyboard function name. #

Total comments: 2

Patch Set 7 : Extended the navigation support on ContentEditable element inside form. #

Total comments: 18

Patch Set 8 : Corrected the eventlistener identification till topmost element. #

Total comments: 4

Patch Set 9 : Refined the Key Event listener finding method. #

Patch Set 10 : Added test coverage for Smart GO/NEXT. #

Total comments: 10

Patch Set 11 : Fixed review comments in unit tests. #

Total comments: 32

Patch Set 12 : #

Patch Set 13 : Fixed the test failure after the refined tests. #

Total comments: 6

Patch Set 14 : Added tabindex and disabled element coverage in unit tests. #

Patch Set 15 : Rebased the patch to solve build breaks in Trybot. #

Patch Set 16 : Fixed unit test failures related tabindex. #

Total comments: 6

Patch Set 17 : Splitted the tests for easy readability. #

Patch Set 18 : Fixed build breaks in disabled element form. #

Total comments: 24

Patch Set 19 : Made all test case as deterministic with EXPECT_EQ #

Total comments: 12

Patch Set 20 : Removed duplicate code in tests using arrays. #

Patch Set 21 : Fixed Windows platform build failure. #

Total comments: 4

Patch Set 22 : Changed while loop to for loop. #

Total comments: 1

Patch Set 23 : Fixed unit test breaks due to one element skip. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+455 lines, -1 line) Patch
M Source/web/WebViewImpl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +3 lines, -0 lines 0 comments Download
M Source/web/WebViewImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 3 chunks +71 lines, -0 lines 0 comments Download
M Source/web/tests/WebViewTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +314 lines, -0 lines 0 comments Download
A Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +11 lines, -0 lines 0 comments Download
A Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +33 lines, -0 lines 0 comments Download
A Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 1 chunk +12 lines, -0 lines 0 comments Download
M public/web/WebTextInputType.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +4 lines, -1 line 0 comments Download
M public/web/WebView.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +7 lines, -0 lines 0 comments Download

Messages

Total messages: 129 (31 generated)
AKV
@bcwhite PTAL blink side changes.
5 years, 10 months ago (2015-02-18 16:22:59 UTC) #2
AKV
@bcwhite, aelias and darin PTAL blink side changes.
5 years, 10 months ago (2015-02-24 14:08:41 UTC) #4
bcwhite
Flags look fine. I'll leave the Blink analysis to the pros. https://codereview.chromium.org/939603002/diff/1/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): ...
5 years, 10 months ago (2015-02-24 17:06:23 UTC) #5
aelias_OOO_until_Jul13
I'm not an editing expert, adding yosin@ for review.
5 years, 10 months ago (2015-02-24 20:27:04 UTC) #7
yosin_UTC9
Please consider to use FocusControl to detect/move input focus. I think users want to stop ...
5 years, 10 months ago (2015-02-25 01:25:37 UTC) #8
yosin_UTC9
+kochi@, he is focus management expert.
5 years, 10 months ago (2015-02-25 01:26:24 UTC) #10
kochi
I'm not sure about the chromium side change, but do Androids need 'hasNext(Previous)InputField' flag? (I ...
5 years, 10 months ago (2015-02-25 03:33:27 UTC) #11
yosin_UTC9
On 2015/02/25 03:33:27, Takayoshi Kochi wrote: > I'm not sure about the chromium side change, ...
5 years, 10 months ago (2015-02-25 04:26:43 UTC) #12
AKV
On 2015/02/25 03:33:27, Takayoshi Kochi wrote: > I'm not sure about the chromium side change, ...
5 years, 10 months ago (2015-02-25 11:21:22 UTC) #13
AKV
On 2015/02/25 04:26:43, Yosi_UTC9 wrote: > On 2015/02/25 03:33:27, Takayoshi Kochi wrote: > > I'm ...
5 years, 10 months ago (2015-02-25 11:34:10 UTC) #14
kochi
On 2015/02/25 11:34:10, AKV wrote: > On 2015/02/25 04:26:43, Yosi_UTC9 wrote: > > On 2015/02/25 ...
5 years, 10 months ago (2015-02-26 04:28:17 UTC) #15
yosin_UTC9
On 2015/02/26 04:28:17, Takayoshi Kochi wrote: > On 2015/02/25 11:34:10, AKV wrote: > > On ...
5 years, 10 months ago (2015-02-26 05:03:21 UTC) #16
kochi
On 2015/02/26 05:03:21, Yosi_UTC9 wrote: > On 2015/02/26 04:28:17, Takayoshi Kochi wrote: > > On ...
5 years, 10 months ago (2015-02-26 07:09:32 UTC) #17
kochi
On 2015/02/26 07:09:32, Takayoshi Kochi wrote: > On 2015/02/26 05:03:21, Yosi_UTC9 wrote: > > On ...
5 years, 8 months ago (2015-04-06 05:01:33 UTC) #18
AKV
On 2015/04/06 05:01:33, Takayoshi Kochi wrote: > On 2015/02/26 07:09:32, Takayoshi Kochi wrote: > > ...
5 years, 8 months ago (2015-04-08 10:06:57 UTC) #19
AKV
@Kochi and Yosi PTAL https://codereview.chromium.org/939603002/diff/1/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/1/Source/web/WebViewImpl.cpp#newcode2454 Source/web/WebViewImpl.cpp:2454: HTMLFormControlElement* formControlElement = toHTMLFormControlElement(element); On ...
5 years, 8 months ago (2015-04-08 18:38:12 UTC) #20
kochi
https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp File Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp#newcode722 Source/core/page/FocusController.cpp:722: Node* FocusController::findFocusableNode(WebFocusType type, Node* stayWithin, Node& node) Do you ...
5 years, 8 months ago (2015-04-09 05:44:38 UTC) #21
AKV
@Kochi - Thanks for the review. Please see my inline comment. https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp File Source/core/page/FocusController.cpp (right): ...
5 years, 8 months ago (2015-04-09 06:16:21 UTC) #22
kochi
https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp File Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp#newcode722 Source/core/page/FocusController.cpp:722: Node* FocusController::findFocusableNode(WebFocusType type, Node* stayWithin, Node& node) On 2015/04/09 ...
5 years, 8 months ago (2015-04-10 02:33:50 UTC) #23
AKV
https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp File Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp#newcode722 Source/core/page/FocusController.cpp:722: Node* FocusController::findFocusableNode(WebFocusType type, Node* stayWithin, Node& node) On 2015/04/10 ...
5 years, 8 months ago (2015-04-10 06:19:13 UTC) #24
kochi
https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp File Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/939603002/diff/20001/Source/core/page/FocusController.cpp#newcode722 Source/core/page/FocusController.cpp:722: Node* FocusController::findFocusableNode(WebFocusType type, Node* stayWithin, Node& node) On 2015/04/10 ...
5 years, 8 months ago (2015-04-10 06:26:21 UTC) #25
AKVT
@Kochi PTAL
5 years, 8 months ago (2015-04-10 14:52:56 UTC) #27
kochi
Sorry for the delay for this review. https://codereview.chromium.org/939603002/diff/40001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/40001/Source/web/WebViewImpl.cpp#newcode2551 Source/web/WebViewImpl.cpp:2551: // TODO(AKV) ...
5 years, 8 months ago (2015-04-14 05:08:31 UTC) #29
AKV
@Kochi - Thanks for the review. I did the changes. PTAL https://codereview.chromium.org/939603002/diff/40001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): ...
5 years, 8 months ago (2015-04-14 06:01:47 UTC) #30
kochi
Thanks, lgtm Does any test from chromium side (browser_tests?) cover this code path?
5 years, 8 months ago (2015-04-14 06:06:47 UTC) #31
AKV
On 2015/04/14 06:06:47, Takayoshi Kochi wrote: > Thanks, lgtm > > Does any test from ...
5 years, 8 months ago (2015-04-14 06:08:37 UTC) #32
kochi
On 2015/04/14 06:08:37, AKV wrote: > On 2015/04/14 06:06:47, Takayoshi Kochi wrote: > > Thanks, ...
5 years, 8 months ago (2015-04-14 06:14:23 UTC) #33
AKV
On 2015/04/14 06:14:23, Takayoshi Kochi wrote: > On 2015/04/14 06:08:37, AKV wrote: > > On ...
5 years, 8 months ago (2015-04-14 06:20:38 UTC) #34
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/60001
5 years, 8 months ago (2015-04-14 06:28:53 UTC) #36
kochi
You still need l-g-t-m from someone in Source/web/OWNERS.
5 years, 8 months ago (2015-04-14 06:32:16 UTC) #37
AKV
On 2015/04/14 06:32:16, Takayoshi Kochi wrote: > You still need l-g-t-m from someone in Source/web/OWNERS. ...
5 years, 8 months ago (2015-04-14 06:33:40 UTC) #38
AKV
@Rick Byers/Darin/Aelias/Adamk PTAL this change.
5 years, 8 months ago (2015-04-14 06:59:23 UTC) #41
Rick Byers
On 2015/04/14 06:59:23, AKV wrote: > @Rick Byers/Darin/Aelias/Adamk PTAL this change. Asking multiple people to ...
5 years, 8 months ago (2015-04-14 14:11:17 UTC) #43
AKV
@tkent PTAL this change.
5 years, 8 months ago (2015-04-14 14:24:18 UTC) #44
AKV
@jochen PTAL this change.
5 years, 8 months ago (2015-04-14 18:21:20 UTC) #46
tkent
I'm an appropriate reviewer. Remove jochen@.
5 years, 8 months ago (2015-04-14 23:02:07 UTC) #48
tkent
https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp#newcode2535 Source/web/WebViewImpl.cpp:2535: Element* WebViewImpl::haveNextInput(Element* element, WebFocusType focusType) The function name is ...
5 years, 8 months ago (2015-04-14 23:27:20 UTC) #49
kochi
https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp#newcode2547 Source/web/WebViewImpl.cpp:2547: if (!nextNode->isDescendantOf(parentFormNode)) On 2015/04/14 23:27:19, tkent wrote: > This ...
5 years, 8 months ago (2015-04-15 01:09:18 UTC) #50
jdduke (slow)
We should not land this until the upstream change has been tested and approved.
5 years, 8 months ago (2015-04-15 18:34:46 UTC) #52
jdduke (slow)
On 2015/04/15 18:34:46, jdduke wrote: > We should not land this until the upstream change ...
5 years, 8 months ago (2015-04-15 18:35:12 UTC) #53
AKV
On 2015/04/15 18:35:12, jdduke wrote: > On 2015/04/15 18:34:46, jdduke wrote: > > We should ...
5 years, 8 months ago (2015-04-15 18:45:34 UTC) #54
jdduke (slow)
On 2015/04/15 18:45:34, AKV wrote: > On 2015/04/15 18:35:12, jdduke wrote: > > On 2015/04/15 ...
5 years, 8 months ago (2015-04-15 21:30:29 UTC) #55
AKV
@tkent & kochi PTAL. https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp#newcode2535 Source/web/WebViewImpl.cpp:2535: Element* WebViewImpl::haveNextInput(Element* element, WebFocusType focusType) ...
5 years, 8 months ago (2015-04-16 19:05:19 UTC) #56
AKV
@tkent & kochi PTAL Patch Set 6!
5 years, 8 months ago (2015-04-16 19:53:34 UTC) #57
kochi
lgtm for the focus control part. Defer anything else to tkent.
5 years, 8 months ago (2015-04-17 01:40:04 UTC) #58
tkent
https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp#newcode2537 Source/web/WebViewImpl.cpp:2537: if (!element->isFormControlElement()) On 2015/04/16 19:05:19, AKV wrote: > On ...
5 years, 8 months ago (2015-04-17 04:12:35 UTC) #59
AKV
@tkent PTAL https://codereview.chromium.org/939603002/diff/100001/public/web/WebView.h File public/web/WebView.h (right): https://codereview.chromium.org/939603002/diff/100001/public/web/WebView.h#newcode190 public/web/WebView.h:190: virtual void advanceFocusToNextInputField(WebFocusType focusType) { } On ...
5 years, 8 months ago (2015-04-17 10:16:41 UTC) #60
AKV
On 2015/04/17 04:12:35, tkent wrote: > https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp > File Source/web/WebViewImpl.cpp (right): > > https://codereview.chromium.org/939603002/diff/60001/Source/web/WebViewImpl.cpp#newcode2537 > ...
5 years, 8 months ago (2015-04-17 10:20:07 UTC) #61
tkent
This change needs tests. > > BTW, If we don't show the enter key in ...
5 years, 8 months ago (2015-04-20 01:40:21 UTC) #62
AKV
On 2015/04/20 01:40:21, tkent wrote: > This change needs tests. >>>>>>>> > Tests at Java ...
5 years, 8 months ago (2015-04-20 10:27:05 UTC) #63
AKV
@tkent PTAL https://codereview.chromium.org/939603002/diff/120001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/120001/Source/web/WebViewImpl.cpp#newcode2543 Source/web/WebViewImpl.cpp:2543: Element* parent = element; On 2015/04/20 01:40:20, ...
5 years, 8 months ago (2015-04-20 19:46:49 UTC) #64
tkent
> > This change needs tests. > > >>>>>>>> > > Tests at Java layer ...
5 years, 8 months ago (2015-04-21 07:33:50 UTC) #65
AKV
On 2015/04/21 07:33:50, tkent wrote: > > > This change needs tests. > > > ...
5 years, 8 months ago (2015-04-21 07:38:44 UTC) #66
tkent
https://codereview.chromium.org/939603002/diff/140001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/140001/Source/web/WebViewImpl.cpp#newcode2589 Source/web/WebViewImpl.cpp:2589: hasKeyEventListener = formOwner->hasEventListeners(EventTypeNames::keydown) || formOwner->hasEventListeners(EventTypeNames::keypress) || formOwner->hasEventListeners(EventTypeNames::keyup); You don't ...
5 years, 8 months ago (2015-04-21 07:41:27 UTC) #67
tkent
On 2015/04/21 07:38:44, AKV wrote: > @tkent - Thanks for your reviews. Do I need ...
5 years, 8 months ago (2015-04-21 07:45:49 UTC) #68
AKV
https://codereview.chromium.org/939603002/diff/140001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/140001/Source/web/WebViewImpl.cpp#newcode2589 Source/web/WebViewImpl.cpp:2589: hasKeyEventListener = formOwner->hasEventListeners(EventTypeNames::keydown) || formOwner->hasEventListeners(EventTypeNames::keypress) || formOwner->hasEventListeners(EventTypeNames::keyup); On 2015/04/21 ...
5 years, 8 months ago (2015-04-21 07:52:59 UTC) #69
AKV
On 2015/04/21 07:45:49, tkent wrote: > On 2015/04/21 07:38:44, AKV wrote: > > @tkent - ...
5 years, 8 months ago (2015-04-21 09:25:57 UTC) #70
AKV
On 2015/04/21 07:45:49, tkent wrote: > On 2015/04/21 07:38:44, AKV wrote: > > @tkent - ...
5 years, 8 months ago (2015-04-27 14:14:55 UTC) #71
AKV
@tkent PTAL
5 years, 7 months ago (2015-04-28 15:01:11 UTC) #72
tkent
Please add tests.
5 years, 7 months ago (2015-04-29 23:38:12 UTC) #73
AKV
On 2015/04/29 23:38:12, Slow until end of May wrote: > Please add tests. @tkent - ...
5 years, 7 months ago (2015-04-30 11:51:19 UTC) #74
AKV
@tkent & kochi PTAL
5 years, 5 months ago (2015-06-30 14:40:38 UTC) #75
kochi
Welcome back. First round of comments. https://codereview.chromium.org/939603002/diff/180001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/180001/Source/web/tests/WebViewTest.cpp#newcode3061 Source/web/tests/WebViewTest.cpp:3061: Element* elementList[20] = ...
5 years, 5 months ago (2015-07-02 11:04:36 UTC) #76
AKV
@tkent Thanks for your review comments. PTAL new patch. https://codereview.chromium.org/939603002/diff/180001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/180001/Source/web/tests/WebViewTest.cpp#newcode3061 Source/web/tests/WebViewTest.cpp:3061: ...
5 years, 5 months ago (2015-07-02 12:45:05 UTC) #77
kochi
Here are second round of comments. https://codereview.chromium.org/939603002/diff/200001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/200001/Source/web/WebViewImpl.cpp#newcode125 Source/web/WebViewImpl.cpp:125: #include "public/platform/WebFocusType.h" It's ...
5 years, 5 months ago (2015-07-03 04:10:35 UTC) #78
AKV
@kochi & tkent PTAL. Please address my queries as well. Thank you. https://codereview.chromium.org/939603002/diff/200001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp ...
5 years, 5 months ago (2015-07-26 15:56:34 UTC) #80
kochi
https://codereview.chromium.org/939603002/diff/200001/Source/web/tests/data/advance_focus_in_form.html File Source/web/tests/data/advance_focus_in_form.html (right): https://codereview.chromium.org/939603002/diff/200001/Source/web/tests/data/advance_focus_in_form.html#newcode4 Source/web/tests/data/advance_focus_in_form.html:4: <input type="text" name="input1" id="input1" form="form1" value="input1 from form1"/><br> On ...
5 years, 4 months ago (2015-07-28 08:43:03 UTC) #81
AKV
@kochi and tkent PTAL! https://codereview.chromium.org/939603002/diff/200001/Source/web/tests/data/advance_focus_in_form.html File Source/web/tests/data/advance_focus_in_form.html (right): https://codereview.chromium.org/939603002/diff/200001/Source/web/tests/data/advance_focus_in_form.html#newcode4 Source/web/tests/data/advance_focus_in_form.html:4: <input type="text" name="input1" id="input1" form="form1" ...
5 years, 4 months ago (2015-08-05 08:55:56 UTC) #83
kochi
https://codereview.chromium.org/939603002/diff/320001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/320001/Source/web/tests/WebViewTest.cpp#newcode3033 Source/web/tests/WebViewTest.cpp:3033: { This test has become very long... Could you ...
5 years, 4 months ago (2015-08-05 10:15:13 UTC) #84
AKV
@kochi PTAL https://codereview.chromium.org/939603002/diff/320001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/320001/Source/web/tests/WebViewTest.cpp#newcode3033 Source/web/tests/WebViewTest.cpp:3033: { On 2015/08/05 10:15:13, Takayoshi Kochi wrote: ...
5 years, 4 months ago (2015-08-05 13:42:50 UTC) #85
kochi
I appreciate the quick turnaround. We are almost there. https://codereview.chromium.org/939603002/diff/360001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/360001/Source/web/tests/WebViewTest.cpp#newcode3106 Source/web/tests/WebViewTest.cpp:3106: ...
5 years, 4 months ago (2015-08-06 02:21:00 UTC) #86
AKV
@kochi PTAL https://codereview.chromium.org/939603002/diff/360001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/360001/Source/web/tests/WebViewTest.cpp#newcode3106 Source/web/tests/WebViewTest.cpp:3106: EXPECT_NE(defaultTextInputFlags | WebTextInputFlagHaveNextFocusableElement | WebTextInputFlagHavePreviousFocusableElement, textInputInfo.flags); On ...
5 years, 4 months ago (2015-08-06 08:39:33 UTC) #87
kochi
lgtm Please also get this reviewed by tkent (OWNERS).
5 years, 4 months ago (2015-08-06 10:10:55 UTC) #88
AKV
On 2015/08/06 10:10:55, Takayoshi Kochi wrote: > lgtm > > Please also get this reviewed ...
5 years, 4 months ago (2015-08-06 10:13:25 UTC) #89
tkent
https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp#newcode3036 Source/web/tests/WebViewTest.cpp:3036: WebView* webViewImpl = m_webViewHelper.initializeAndLoad(m_baseURL + testFile, true, 0); The ...
5 years, 4 months ago (2015-08-07 01:40:58 UTC) #90
AKV
@tkent - Could you pls address my queries ? https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp#newcode3052 Source/web/tests/WebViewTest.cpp:3052: ...
5 years, 4 months ago (2015-08-07 06:56:53 UTC) #91
tkent
https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp#newcode3052 Source/web/tests/WebViewTest.cpp:3052: Element* contenteditable1 = document->getElementById("contenteditable1"); On 2015/08/07 06:56:53, AKV wrote: ...
5 years, 4 months ago (2015-08-07 07:05:48 UTC) #92
AKV
@tkent PTAL https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp File Source/web/tests/WebViewTest.cpp (right): https://codereview.chromium.org/939603002/diff/380001/Source/web/tests/WebViewTest.cpp#newcode3036 Source/web/tests/WebViewTest.cpp:3036: WebView* webViewImpl = m_webViewHelper.initializeAndLoad(m_baseURL + testFile, true, ...
5 years, 4 months ago (2015-08-08 09:25:03 UTC) #93
tkent
lgtm
5 years, 4 months ago (2015-08-10 00:38:58 UTC) #95
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/400001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/939603002/400001
5 years, 4 months ago (2015-08-10 00:39:12 UTC) #97
commit-bot: I haz the power
The author ajith.v@chromium.org has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign ...
5 years, 4 months ago (2015-08-10 00:39:15 UTC) #99
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/400001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/939603002/400001
5 years, 4 months ago (2015-08-10 07:50:02 UTC) #101
commit-bot: I haz the power
The author ajith.v@chromium.org has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign ...
5 years, 4 months ago (2015-08-10 07:50:04 UTC) #103
AKV
On 2015/08/10 07:50:04, commit-bot: I haz the power wrote: > The author mailto:ajith.v@chromium.org has not ...
5 years, 4 months ago (2015-08-11 06:54:53 UTC) #104
lgombos
> I have started the CLA for ajith.v@chromium.org id. > Once it's approved I will ...
5 years, 4 months ago (2015-08-11 15:40:18 UTC) #106
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/400001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/939603002/400001
5 years, 4 months ago (2015-08-12 06:40:09 UTC) #108
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_compile_dbg on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/win_blink_compile_dbg/builds/53573)
5 years, 4 months ago (2015-08-12 07:10:10 UTC) #110
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/420001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/939603002/420001
5 years, 4 months ago (2015-08-12 10:24:10 UTC) #113
kochi
https://codereview.chromium.org/939603002/diff/420001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/420001/Source/web/WebViewImpl.cpp#newcode2554 Source/web/WebViewImpl.cpp:2554: return nullptr; How about using for()? for (Element* nextElement ...
5 years, 4 months ago (2015-08-12 10:47:33 UTC) #114
AKV
@kochi - PTAL Thanks. https://codereview.chromium.org/939603002/diff/420001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/420001/Source/web/WebViewImpl.cpp#newcode2554 Source/web/WebViewImpl.cpp:2554: return nullptr; On 2015/08/12 10:47:33, ...
5 years, 4 months ago (2015-08-12 11:10:14 UTC) #116
kochi
lgtm
5 years, 4 months ago (2015-08-12 11:11:10 UTC) #117
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/440001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/939603002/440001
5 years, 4 months ago (2015-08-12 11:12:12 UTC) #120
commit-bot: I haz the power
Try jobs failed on following builders: mac_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/65919)
5 years, 4 months ago (2015-08-12 11:27:35 UTC) #122
kochi
https://codereview.chromium.org/939603002/diff/440001/Source/web/WebViewImpl.cpp File Source/web/WebViewImpl.cpp (right): https://codereview.chromium.org/939603002/diff/440001/Source/web/WebViewImpl.cpp#newcode2550 Source/web/WebViewImpl.cpp:2550: for (Element* nextElement = element; nextElement; nextElement = page()->focusController().findFocusableElement(focusType, ...
5 years, 4 months ago (2015-08-12 11:49:48 UTC) #123
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/939603002/460001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/939603002/460001
5 years, 4 months ago (2015-08-12 14:13:52 UTC) #126
commit-bot: I haz the power
Committed patchset #23 (id:460001) as https://src.chromium.org/viewvc/blink?view=rev&revision=200398
5 years, 4 months ago (2015-08-12 15:40:27 UTC) #127
AKV
On 2015/08/12 15:40:27, commit-bot: I haz the power wrote: > Committed patchset #23 (id:460001) as ...
5 years, 4 months ago (2015-08-12 15:59:14 UTC) #128
tkent
5 years, 4 months ago (2015-08-21 00:39:28 UTC) #129
Message was sent while issue was closed.
A revert of this CL (patchset #23 id:460001) has been created in
https://codereview.chromium.org/1303123002/ by tkent@chromium.org.

The reason for reverting is: Regression in power.android_acceptance.
crbug.com/520952
.

Powered by Google App Engine
This is Rietveld 408576698